So when we are shutting down nsHttpConnectionMng and nsSocketTransportService it would be good to stop opening new connection. Loooking at: https://wiki.mozilla.org/XPCOM_Shutdown On profile-change-net-teardown we shutdown nsHttpConnectionMng and then nsSocketTransportService, but already at this moment it would be good not to do PR_Connect for example. We do have checks for gIOService->IsOffline() but this is true too late, only on xpcom-shutdown notification when nsHttpConnectionMng and nsSocketTransportService are already shutdown. This could help with bug 1158189 and 1152046.

So this is a bit stupid that profile-change-net-teardown is the first notification and we should tear down all network activities in order: tell nsIOService that we are shutting down, shut nsHttpConnectionMng and then shut nsSocketTransportService(which is done by nsIOService) I thought of moving nsHttpHandler shutdown to be done by nsIOService, but there are one on child process a well :) Probably a stupid question, but do we need nsConnectionMgr on the child process?

+1 on the overall thing > Probably a stupid question, but do we need nsConnectionMgr on the child > process? basically no - and I believe I added an assert somewhere that we don't do real networking on the child - but I think jason left it in for a reason..

dragana are you planning a patch for this? I think it would be good

(In reply to Patrick McManus [:mcmanus] from comment #3) > dragana are you planning a patch for this? I think it would be good Yes. My idea is to not initialize nsConnectionMgr on child process and to have it only on parent, of course if we do not need it on the child, so then I can move closing of nsConnectionMgr to the nsIOService. And solution for this problem would look nicer. If this does not work I will need to signal nsIOService from nsHttpHanndler that we are going to tear down networking - not so nice solution.

The only reason we have a socket transport thread on the child is because we need it to run xpcshell tests (and maybe mochitests?) that use httpd.js. I know it has something to do with how we inject server-side handlers into httpd.js, but I don't know whether that means we need nsConnectionMgr or not.

Jason, do we need nsConnectionMgr on the child process? If we need it I will make different solution, if we do not need it you can review.

(In reply to Jason Duell [:jduell] (needinfo? me) from comment #5) > The only reason we have a socket transport thread on the child is because we > need it to run xpcshell tests (and maybe mochitests?) that use httpd.js. I > know it has something to do with how we inject server-side handlers into > httpd.js, but I don't know whether that means we need nsConnectionMgr or not. I somehow missed your comment. I will do a try run.

I have only change commit message here (the patch is the same as the one pushed to try server). So try server have not complained about not starting nsConnectionMgr on child process, so I am asking for a review.

Comment on attachment 8707329 [details] [diff] [review] bug_1238910_v1.patch Review of attachment 8707329 [details] [diff] [review]: ----------------------------------------------------------------- ::: netwerk/base/nsSocketTransport2.cpp @@ -1221,5 @@ > bool isLocal; > IsLocal(&isLocal); > > - if (gIOService->IsShutdown()) { > - return NS_ERROR_ABORT; why the change from abort to offline here?

(In reply to Patrick McManus [:mcmanus] from comment #10) > Comment on attachment 8707329 [details] [diff] [review] > bug_1238910_v1.patch > > Review of attachment 8707329 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: netwerk/base/nsSocketTransport2.cpp > @@ -1221,5 @@ > > bool isLocal; > > IsLocal(&isLocal); > > > > - if (gIOService->IsShutdown()) { > > - return NS_ERROR_ABORT; > > why the change from abort to offline here? That was my mistake: I thought that profile-change-net-teardown is fired not only during shutdown but in some other cases too. So I did not want to kill local connections for "other" cases. I checked: it is fired only on shutdown so I will change this to ABORT all.