The JS component loader leaks when JS components are loaded. There are 2 causes for the leak, one of which I don't fully understand yet: 1) The XPConnect singleton gets shutdown before the JS component loader. This can be fixed by having the JS component loader own a reference to the XPConnect singleton. 2) The call to CanUnload in UnloadAndReleaseModules returns NS_ERROR_FAILURE, so the module doesn't get released. The leak can be fixed by releasing anyway if CanUnload returns a failure status, but I'm still looking for a way to fix the failure return.

> 1) The XPConnect singleton gets shutdown before the JS component loader. > This can be fixed by having the JS component loader own a reference to the > XPConnect singleton. No. We've been over this ground before. Delaying xpconnect shutdown causes too many bad things to happen throughout the application during shutdown of various services. When xpconnect shuts down it sets up walls so that JS code and native code can't call either way through xpconnect. We have found many times that by delaying this we can fix some shutdown leaks, but the cost turns out to be calls into code *all* over the place* that is in an unstable state because the service manager is by then not vending services and the various poormans's weakreferences that so much code relies on have become dangling pointers. We trade shutdown leaks for shutdown crashes. I'll go with the leaks. Maybe I'm being a luddite here, but I don't think we can safely assume that we can fix *all* these kinds of things. there is too *stuff* out there and too much uncertainly of shutdown order etc. It may be that we can get some relief by adding a system to control from xpcom the shutdown order of component loaders and well known services.

OK.

Marking Verified -

Reopening so I remember to investigate why we get the NS_ERROR_FAILURE return I mentioned in point (2).

(This leak has become more of an issue again in regular leak detection, and is adding noise, thanks to nsCloseAllWindows.js.) One other possibility here is that mozJSComponentLoader::UnloadAll do unrooting in addition to just calling CanUnload on all the modules. It seems to me that the code in the mozJSComponentLoader destructor will never do anything useful since it is guaranteed to be run after XPConnect shutdown (at the earliest, from the loop to release the loaders in nsComponentManagerImpl::Shutdown, which is after module destructors are called). I don't think this violates either of the rules that (1) XPConnect (as is any module (now, anyway) implementing features allowing modules to be implemented in non-native code) is just a normal native code module and (2) once we've started to unload modules, we shouldn't propagate any calls across modules, since the non-native component loaders' UnloadAll methods are called (see nsComponentManagerImpl::UnloadLibraries) before any of the native modules are unloaded. But actually, thinking through this makes me notice a few problems: * The loop in nsComponentManagerImpl::Shutdown to release the loaders is happening too late, since it's propagating releases into native code modules after module shutdown (which is when we would be unloading libraries if we unloaded libraries). (This, I think, could be fixed by folding the releases of the loaders into UnloadAll itself, rather than doing them in a separate loop in Shutdown.) * nsComponentManagerImpl::UnloadAll doesn't consider mStaticComponentLoader specially, but it probably needs to.

This is sufficient to fix the shutdown leaks, but it doesn't fix the problems I mentioned in my previous comment.