src: keep main-thread Isolate attached to platform during Dispose

addaleax · codebytere · commit a095ef0d52a4 · 2020-02-17T12:49:17.000-08:00
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>
diff --git a/src/node_main_instance.cc b/src/node_main_instance.cc
@@ -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() {

Original file line number	Diff line number	Diff line change
`@@ -102,8 +102,12 @@ NodeMainInstance::~NodeMainInstance() {`
`102`	`102`	`if (!owns_isolate_) {`
`103`	`103`	`return;`
`104`	`104`	`}`
`105`		`- platform_->UnregisterIsolate(isolate_);`
	`105`	`+ // TODO(addaleax): Reverse the order of these calls. The fact that we first`
	`106`	`+ // dispose the Isolate is a temporary workaround for`
	`107`	`+ // https://github.com/nodejs/node/issues/31752 -- V8 should not be posting`
	`108`	`+ // platform tasks during Dispose(), but it does in some WASM edge cases.`
`106`	`109`	`isolate_->Dispose();`
	`110`	`+ platform_->UnregisterIsolate(isolate_);`
`107`	`111`	`}`
`108`	`112`
`109`	`113`	`int NodeMainInstance::Run() {`