From e460f8cf43863a5a8d73273ce311135ad3245699 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Fri, 14 Feb 2020 16:28:58 +0100 Subject: [PATCH] src: keep main-thread Isolate attached to platform during Dispose MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 25447d82d cannot occur for the `NodeMainInstance`’s Isolate, as it is the last one to exit in any given Node.js process. This partially reverts 25447d82d. Refs: https://github.com/nodejs/node/pull/30909 Refs: https://github.com/nodejs/node/issues/31752 PR-URL: https://github.com/nodejs/node/pull/31795 Reviewed-By: Denys Otrishko Reviewed-By: James M Snell --- src/node_main_instance.cc | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/node_main_instance.cc b/src/node_main_instance.cc index ad6966cf4fc13e..d53eaa7329beed 100644 --- 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() {