From 6bc6746a6745e37c4de10b4866ee3861e1f138d0 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Tue, 12 Dec 2023 21:43:32 +0100 Subject: [PATCH] src: enter isolate before destructing IsolateData MVP fix for a worker_threads crash where ~WorkerThreadData() -> ~IsolateData() -> Isolate::DetachCppHeap() kicks off a round of garbage collection that expects an entered isolate. No test because the crash is not reliably reproducable but the bug is pretty clearly described in the linked issue and is obvious once you see it, IMO. Fixes: https://github.com/nodejs/node/issues/51129 --- src/node_worker.cc | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/node_worker.cc b/src/node_worker.cc index 900674bbe4c90e..0affbe7c85a013 100644 --- a/src/node_worker.cc +++ b/src/node_worker.cc @@ -217,7 +217,13 @@ class WorkerThreadData { CHECK(!loop_init_failed_); bool platform_finished = false; - isolate_data_.reset(); + // https://github.com/nodejs/node/issues/51129 - IsolateData destructor + // can kick off GC before teardown, so ensure the isolate is entered. + { + Locker locker(isolate); + Isolate::Scope isolate_scope(isolate); + isolate_data_.reset(); + } w_->platform_->AddIsolateFinishedCallback(isolate, [](void* data) { *static_cast(data) = true;