Closed Bug 1202468 Opened 4 years ago Closed 4 years ago
Socket Transport Service::m Pending Socket Q is locked unnecessarily
AFAICT, mPendingSocketQ is only ever accessed on the socket transport thread: https://dxr.mozilla.org/mozilla-central/search?q=mPendingSocketQ&redirect=true&case=true&limit=99&offset=0 NotifyWhenCanAttachSocket, where events are added to the queue, has an explicit assertion that we are on the socket thread. DetachSocket, where events are removed from the queue, has no such assertion, but a try run with such an assertion added does not result in failures. The comment hereabouts: http://mxr.mozilla.org/mozilla-central/source/netwerk/base/nsSocketTransportService2.cpp#188 suggests that the following functions are only supposed to be called on the socket thread. Tracing through the call chains suggests that Detach Socket is indeed only called on the socket thread. We should remove this unnecessary locking, either by refactoring nsEventQueue, or finding a different data structure to use.
I think you're right that the locked class is un-necessary. I also think this code path is pretty unusual (its an overflow kind of thing) and the removing the lock won't impact most cases.
Assignee: nobody → mcmanus
Attachment #8755490 - Flags: review?(dd.mozilla) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/fd50e33b4e9b825b4634fc79db5631defa3b88b7 Bug 1202468 - remove mPendingQ lock r=dragana
You need to log in before you can comment on or make changes to this bug.