Closed Bug 1414443 Opened 2 years ago Closed 2 years ago

js --help gives "Assertion failure: shutdown" due to BufferStreamState destructor

Categories

(Core :: JavaScript Engine, enhancement)

enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla58
Tracking Status
firefox58 --- fixed

People

(Reporter: sfink, Assigned: sfink)

Details

Attachments

(1 file, 2 obsolete files)

I don't *think* this is due to any of my patches, anyway...

bufferStreamState is a file-scoped variable whose destructor depends on ShutdownBufferStreams() having been called, but that is only called near the end of main() and there are a number of ways that main() could return early (the most plausible having to do with option parsing.) Running with --help is an easy way to hit this case.
This probably isn't quite right either, but it makes sure ShutdownBufferStreams gets called. Note that --help will still skip the other main() shutdown calls, like:

    JS_SetGrayGCRootsTracer(cx, nullptr, nullptr);
    sc->markObservers.reset();
    KillWatchdog(cx);
    KillWorkerThreads(cx);
    DestructSharedArrayBufferMailbox();
    JS_DestroyContext(cx);
    JS_ShutDown();

so it might be better for main() to do a ScopeExit trick for all of those too, right after the setup. But that wouldn't necessarily fix this one, because bufferStreamState's scope is outside of main() already. Or maybe there should be a wrapper function.

I didn't look too deeply at this bufferstreamstatestuff, so I'm not sure what its invariants are. Could it be nested within main()? Perhaps as a Maybe<> that gets emplaced in ConsumeBufferSource or something?
Attachment #8925180 - Flags: review?(luke)
Oops, clang did not like the lambda capturing the whole toplevel scope or something:

error: non-local lambda expression cannot have a capture-default
Attachment #8925185 - Flags: review?(luke)
Attachment #8925180 - Attachment is obsolete: true
Attachment #8925180 - Flags: review?(luke)
Attempt 3: several of the builds (clang-based, but not all of them) did not like having a ScopeExit<lambda> type at file scope.
Attachment #8925215 - Flags: review?(luke)
Attachment #8925185 - Attachment is obsolete: true
Attachment #8925185 - Flags: review?(luke)
Comment on attachment 8925215 [details] [diff] [review]
Alternative way to shut down buffer streams

Review of attachment 8925215 [details] [diff] [review]:
-----------------------------------------------------------------

d'oh, thanks!
Attachment #8925215 - Flags: review?(luke) → review+
Pushed by sfink@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/338f65684d59
Alternative way to shut down buffer streams, r=luke
https://hg.mozilla.org/mozilla-central/rev/338f65684d59
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
You need to log in before you can comment on or make changes to this bug.