Skip to content

Commit

Permalink
src: keep main-thread Isolate attached to platform during Dispose
Browse files Browse the repository at this point in the history
This works around a situation in which the V8 WASM code calls
into the platform while the Isolate is being disposed.

This goes against the V8 API constract for `v8::Platform`.
In lieu of a proper fix, it should be okay to keep the Isolate
registered; the race condition fixed by 25447d8 cannot
occur for the `NodeMainInstance`’s Isolate, as it is the last
one to exit in any given Node.js process.

This partially reverts 25447d8.

Refs: nodejs#30909
Refs: nodejs#31752

PR-URL: nodejs#31795
Reviewed-By: Denys Otrishko <[email protected]>
Reviewed-By: James M Snell <[email protected]>
  • Loading branch information
addaleax committed Feb 14, 2020
1 parent 75311db commit e460f8c
Showing 1 changed file with 5 additions and 1 deletion.
6 changes: 5 additions & 1 deletion src/node_main_instance.cc
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,12 @@ NodeMainInstance::~NodeMainInstance() {
if (!owns_isolate_) {
return;
}
platform_->UnregisterIsolate(isolate_);
// TODO(addaleax): Reverse the order of these calls. The fact that we first
// dispose the Isolate is a temporary workaround for
// https://github.com/nodejs/node/issues/31752 -- V8 should not be posting
// platform tasks during Dispose(), but it does in some WASM edge cases.
isolate_->Dispose();
platform_->UnregisterIsolate(isolate_);
}

int NodeMainInstance::Run() {
Expand Down

0 comments on commit e460f8c

Please sign in to comment.