Closed Bug 1239961 Opened 4 years ago Closed 4 years ago

Do less PR_Poll during shutdown

Categories

(Core :: Networking, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla46
Tracking Status
firefox46 --- fixed

People

(Reporter: dragana, Assigned: dragana)

References

Details

Attachments

(1 file, 1 obsolete file)

We can also do less PR_Poll during shutdown.

We do not need to do PR_Connect, PR_ConnectContinue and also PR_Poll during shutdown (at least as soon as nsIOService knows that we are in shutdown)

Also nsHttpConnection::Close does PR_Read to take all data from the socket sockets so that we do not cause TCP RST. We could also skip that.

Only thing left should be PR_Close and less calls that can hang :)
Assignee: nobody → dd.mozilla
Status: NEW → ASSIGNED
Blocks: 1152046
Looking at the code, nsHttpConnectionMgr posts nsHttpConnectionMgr::OnMsgShutdown event on socketThread. This needs to be done before socketThread shuts down, but right after that we can start shutting down thread, I think.

nsHttpConnectionMgr::OnMsgShutdown closes all nsHttpConnection so only connections left are non http connection (e.g. udp).
So DoPollIteration coming after that function will detach all socket (we will do that also during socketThread shutdown) and poll for non http connection (I have not look at them jet).
To optimize this I will only prevent PR_Poll call (and let socketThread call detachSocket while main thread is still busy with some other stuff and not do thate later while main thread is waiting for it).
Attached patch bug_1239961_v1.patch (obsolete) — Splinter Review
Attachment #8708286 - Flags: feedback?(mcmanus)
Attachment #8708286 - Attachment is obsolete: true
Attachment #8708286 - Flags: feedback?(mcmanus)
Attachment #8708438 - Flags: feedback?(mcmanus)
Comment on attachment 8708438 [details] [diff] [review]
bug_1239961_v1.patch

Review of attachment 8708438 [details] [diff] [review]:
-----------------------------------------------------------------

I think all of these changes will really help in aggregate. thanks!
Attachment #8708438 - Flags: feedback?(mcmanus) → feedback+
Attachment #8708438 - Flags: review?(mcmanus)
The next step would be to limit number of PR_Close calls.
Something like: if we are in shutdown already for more than 3-4min do not call PR_Close let them leak.
Probably this can be done in: http://mxr.mozilla.org/mozilla-central/source/netwerk/base/nsSocketTransport2.cpp#1734
Attachment #8708438 - Flags: review?(mcmanus) → review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/c12b2d098ae0
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla46
You need to log in before you can comment on or make changes to this bug.