I'll attach a patch showing a crash in JS_DestroyContext for the prefs JSContext that gets triggered from a finalizer run dur to a JS_DestroyRuntime (aka JS_Finish) call. The problem is that the JSContext is question has already been destroyed in JS_DestroyRuntime. I see this after doing a "touch components/*.js". This means that the JS Loader makes a JS context early on and is the last to release the nsJSRuntimeService. The nsJSRuntimeService thiks it 'owns' the lifetime of the whoe JS runtime, yet doesn't have any control over the lifetime of JSContexts - except that JS_DestroyRuntime is the death squad. What to do?
Wait, this proves that we leak JSContexts (duh!) if we don't do that destroy-all-contexts loop in JS_Finish. Which is worse, leaking or crashing? Hmm, if we can avoid crashing (not clear, but possible) by keeping that loop but making it safe, we win. See what I mean? As there are native data structures being leaked, if we clean up the JS side of the leak, maybe some native will have a dangling mScriptObject pointer -- but nsGenericFinalize gives me hope that such JS roots get removed. I think this is a dup of bug 52835. /be
One native data struct behind whose back we don't want to destroy a JSContext is nsJSContext, obviously. I'm a bozo for taking email@example.com's destory-all-contexts loop in JS_Finish, but not his API-incompatible JSContext ref-counting changes. It is obvious that there are two owners now, the caller of JS_NewContext, and the rt->contextList from which destroy-all-contexts finds the victims. So we need at least two bits of ref-count with the code as it stands. But then we'll still leak as we do today, because the 1 ref-count for the cx returned to the caller of JS_NewContext may not get passed to JS_DestroyContext (prefs is good, it doesn't leak, but the DOM may because of webshell/docshell leaks), even if we take care of the other +1 (for 2 total) count for the link from rt->contextList. You can't fix leaks of JSContexts from within the JS engine, magically. I'm duping this against bug 52835 and attaching a proposed fix patch there. /be *** This bug has been marked as a duplicate of 52835 ***
Marking Verified -