Closed Bug 200030 Opened 17 years ago Closed 17 years ago
XPConnect should release wrappers at shutdown
When XPConnect disconnects the wrapper from the JSObject, it should release a reference. This will cleanup any wrappers that no longer have any references. This appears it might be affecting the component manager on shutdown.
This silences Purify's reported leaked wrapped natives. I need to do some investigating to make sure this doesn't have any ill effects. I also need to verify that this benefits the component manager leak. If it doesn't it may not be as important to deal with this right now.
I did some preliminary testing. I removed the oji component, and ran without this patch and the component manager didn't leak. This cleans up some shutdown leaks, but don't think the risk is worth the reward. Removing the block of bug 78861 and moving to 1.5a
No longer blocks: 78861
Target Milestone: --- → mozilla1.5alpha
Pulling this back to 1.4b. I've verified that we're leaking wrapped natives after previous patches have been applied. This gets rid of two of three shutdown leaks in XPConnect. I want to look a little deeper as to why these objects are leaking and make sure that there's not a secondary reason that should be addressed. And I'm reviewing the patch to double check that it's doing the right thing. jband, brendan, others, feel free to take a look now and note any problems. I'll be requesting formal reviews a little later. We'll want to get this in soon so that it has a good amount of time to bake.
What's the difference between what this patch does and reordering ~nsXPConnect? Does this patch make the two comments in ~nsXPConnect invalid?
The first comment, yes. The code I added calls release on the natives during shutdown. Normally this occurs when the object is finalized. It appears to be absent in the shutdown code, and probably was done on purpose. What I'm looking into now is why it was left out, and if the reason is still valid.
"probably was done on purpose" ??? I left a comment in the code to make it clear that this was intended. I admit that the comment does not detail the problems(and I don't really remember all the specific details). http://lxr.mozilla.org/seamonkey/source/js/src/xpconnect/src/xpcwrappednative.cpp#914 What I *used* to see before giving up and not doing the releases, was a range of places outside xpconnect that would do bad stuff upon final release if that release came during xpcom shutdown - when various other parts of the entire system were no longer in their 'normal' operating state. Note that we are talking about fairly random shutdown ordering here. It may well be that many of these cases are long since fixed. I dunno. If you can make this all be stable then great. I long resisted messing with this since I saw perfect shutdown cleanup as far less important than good shutdown stability. And, I just didn't believe that all the wierd assumptions about lifetime all over the code were very fixable. But that's just me. Perhaps the landscape has changed enough since then. Anyway it's not my problem now :)
I was thinking that some of the blocking off of XPCOM's CreateInstance/GetService during shutdown might make this viable again. But I was still worried about other avenues that I wasn't seeing in simple tests. This was the reason I initially pushed it back, but then was asked to take another look to see if I can eliminate it. There's a push to reduce shutdown leaks for embedding, and that's why I was looking into this.
A reason not to release wrappers at shutdown is that it is (or should be) against the rules of XPCOM to propagate releases into other modules during module destructors, since module destructors are at the time of library unloading (if we ever start unloading libraries) and you shouldn't attempt to call code in libraries that have been unloaded. I was attempting to fix the existing shutdown leak (there's really only one, when we don't otherwise leak) related to this issue in bug 180380. I had a patch that worked for a little while, but wasn't really ideal. Probably a better approach would be to somehow disconnect the components object at some point during shutdown so that its wrapper can be GCed before we shutdown XPConnect. (Or perhaps we could make a special exception for the wrapper of the XPCComponents object during shutdown? However, that, like the solution in this bug, would still mean, I think, that a bunch of the JS objects involved would leak, so I don't think we'd gain very much.)
I'm going to go ahead and dupe this against bug 180380 since the goal is the same. *** This bug has been marked as a duplicate of 180380 ***
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → DUPLICATE
Verified Duplicate -
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.