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: #30909
Refs: #31752

PR-URL: #31795
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
addaleax authored and codebytere committed Feb 17, 2020
1 parent a840e9d commit a095ef0
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 a095ef0

Please sign in to comment.