899270 - !JSRuntime::hasLiveRuntimes() fatal assertion breaks nsTraceRefcnt's BloatView

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Jesse Ruderman]

The leak testcase in bug 898906 now trips an assertion on shutdown:

Assertion failure: !JSRuntime::hasLiveRuntimes() (forgot to destroy a runtime before shutting down), at js/src/jsapi.cpp:710

This assertion was introduced in bug 896124:
http://hg.mozilla.org/mozilla-central/rev/e432690bf0fb

Turning a leak into a fatal assertion makes me a little sad.

Comment 1

[Mike de Boer [:mikedeboer]]

As I mentioned in https://bugzilla.mozilla.org/show_bug.cgi?id=896124#c17, this causes a crash for me on startup.

Comment 2

[Mike de Boer [:mikedeboer]]

Start of the browser with -ProfileManager, I just found out! So it's on Profile Manager shutdown that this assertion gets called, which halts the browser startup sequence.

Comment 3

[Jeff Walden [:Waldo]]

This really isn't a JS engine bug.  If the embedder isn't destroying all his runtimes, that's his own fault, and there's really nothing the JS engine can safely do about that.

Comment 4

[Andrew McCreight [:mccr8]]

It isn't okay to turn a shutdown leak into a fatal assertion, because this essentially disables BloatView, which is a critical part of our leak fixing infrastructure.  This makes it harder to tell when there is a leak, harder to fix a leak (because you can't see what is leaking), and it also makes it impossible to see how bad a leak is (is it just leaking a few strings or an entire document?).  This is going to make it impossible to distinguish different leaks on TBPL.

For instance, bug 900636 is "Intermittent Windows mochitest-2 TEST-UNEXPECTED-FAIL | Shutdown | application timed out after 330 seconds with no output (Assertion failure: !JSRuntime::hasLiveRuntimes() (forgot to destroy a runtime before shutting down), at js/src/jsapi.cpp:710)".  You can't even easily tell that it is a shutdown leak, or see anything about what is leaking.

Can we turn this into some kind of non-fatal assertion?  Or failing that, remove the assertion altogether?

Comment 5

[Andrew McCreight [:mccr8]]

Judging by the messages for these two intermittent oranges, we're burning 5.5 minutes of test pool time each time we hit this assertion (waiting for the timeout), rather than just shutting down.

Comment 6

[Jeff Walden [:Waldo]]

Turned into an fprintf(stderr, ...) in a followup push in bug 896124.

We must burn 5.5 minutes every time we crash, then.  Feel free to take up that issue with the test harnesses or something if you want, but that behavior's nothing new at all.

Comment 7

[Jesse Ruderman]

http://hg.mozilla.org/integration/mozilla-inbound/rev/1b9a459cea77