JS crash due to JS_DestroyContext during JS_DestroyRuntime

VERIFIED DUPLICATE of bug 52835

Status

()

Core
XPConnect
P3
normal
VERIFIED DUPLICATE of bug 52835
18 years ago
18 years ago

People

(Reporter: John Bandhauer, Assigned: John Bandhauer)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

18 years ago
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?
(Assignee)

Comment 1

18 years ago
Created attachment 14819 [details]
stack of crash
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 tlundeen@webcrossing.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 ***
Status: NEW → RESOLVED
Last Resolved: 18 years ago
Resolution: --- → DUPLICATE

Comment 4

18 years ago
Marking Verified -
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.