From d76e16bb477d309e81cef047e7a1f91f36eb1c35 Mon Sep 17 00:00:00 2001
From: Ben Noordhuis <info@bnoordhuis.nl>
Date: Mon, 25 Dec 2023 13:12:54 +0100
Subject: [PATCH] src: enter isolate before destructing IsolateData
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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
PR-URL: https://github.com/nodejs/node/pull/51138
Reviewed-By: Santiago Gimeno <santiago.gimeno@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Juan José Arboleda <soyjuanarbol@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
---
 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<bool*>(data) = true;