Closed Bug 1414443 Opened 2 years ago Closed 2 years ago
js --help gives "Assertion failure: shutdown" due to Buffer
Stream State destructor
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?
Oops, clang did not like the lambda capturing the whole toplevel scope or something: error: non-local lambda expression cannot have a capture-default
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)
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 firstname.lastname@example.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/338f65684d59 Alternative way to shut down buffer streams, r=luke
You need to log in before you can comment on or make changes to this bug.