chore: stop leaking v8 environment in main process #22761
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of Change
This changes our shutdown procedure to include cleaning up the v8 Isolate. The bug referenced in the comment appears to no longer be an issue.
Two things broke when I made this change, both discovered via ASan:
Browser
was being destroyed after the Isolate, which meant it was attempting to clean up itsGlobal
handles, but v8 had already cleaned them up.v8::Global
, which were crashing on shutdown because their destructors were being called during C++'s finalization stage. I've switched those globals tobase::NoDestructor
to prevent those destructors from being called. They might be better stored ingin::PerIsolateData
or on some other object with a more clearly defined lifetime. However, at present we only ever have a single v8 isolate in the main process, so that problem can be solved another time.Checklist
npm test
passesRelease Notes
Notes: none