src: track handle cleanup per thread, not per IsolateData

The FreeEnvironment() fix for sibling Environments keeps the depth of nested Environment::CleanupHandles() calls on the IsolateData, so that InternalCallbackScope can re-allow JavaScript for sibling Environments while one of them is being freed. Environments that each have their own IsolateData on the same isolate and loop never see that counter and still fail with "illegal access". Environments that share a loop share a thread, so keep the depth in a thread_local instead. Refs: #65977 Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>
codebytere · Sep 23, 2026 · Sep 22, 2026 · Sep 22, 2026 · Sep 22, 2026 · Sep 22, 2026
commit fece197977b9e25b030d2a0d8c3a581988f09343
diff --git a/src/api/callback.cc b/src/api/callback.cc
@@ -96,8 +96,7 @@ InternalCallbackScope::InternalCallbackScope(
   }
 
   Isolate* isolate = env->isolate();
-  // See IsolateData::handle_cleanup_depth.
-  if (env->isolate_data()->handle_cleanup_depth > 0) allow_js_.emplace(isolate);
+  if (handle_cleanup_depth > 0) allow_js_.emplace(isolate);
 
   HandleScope handle_scope(isolate);
   Local<Context> current_context = isolate->GetCurrentContext();

diff --git a/src/env.cc b/src/env.cc
@@ -1436,6 +1436,8 @@ void Environment::ClosePerEnvHandles() {
   close_and_finish(reinterpret_cast<uv_handle_t*>(&task_queues_async_));
 }
 
+thread_local int handle_cleanup_depth = 0;
+
 void Environment::CleanupHandles() {
   {
     Mutex::ScopedLock lock(native_immediates_threadsafe_mutex_);
@@ -1453,8 +1455,8 @@ void Environment::CleanupHandles() {
   for (HandleWrap* handle : handle_wrap_queue_)
     handle->Close();
 
-  isolate_data()->handle_cleanup_depth++;
-  auto done = OnScopeLeave([&]() { isolate_data()->handle_cleanup_depth--; });
+  handle_cleanup_depth++;
+  auto done = OnScopeLeave([]() { handle_cleanup_depth--; });
   while (handle_cleanup_waiting_ != 0 ||
          request_waiting_ != 0 ||
          !handle_wrap_queue_.IsEmpty()) {

diff --git a/src/env.h b/src/env.h
@@ -182,11 +182,6 @@ class NODE_EXTERN_PRIVATE IsolateData : public MemoryRetainer {
   inline worker::Worker* worker_context() const;
   inline void set_worker_context(worker::Worker* context);
 
-  // Non-zero while an Environment on this isolate is closing its handles with
-  // JS disallowed isolate-wide; InternalCallbackScope re-allows it for the
-  // other Environments whose callbacks run in those loop turns.
-  int handle_cleanup_depth = 0;
-
 #define VP(PropertyName, StringValue) V(v8::Private, PropertyName)
 #define VY(PropertyName, StringValue) V(v8::Symbol, PropertyName)
 #define VS(PropertyName, StringValue) V(v8::String, PropertyName)

diff --git a/src/node_internals.h b/src/node_internals.h
@@ -283,6 +283,11 @@ class InternalCallbackScope {
   std::optional<v8::Isolate::AllowJavascriptExecutionScope> allow_js_;
 };
 
+// Non-zero while an Environment on this thread is closing its handles with JS
+// disallowed isolate-wide; InternalCallbackScope re-allows it for the other
+// Environments whose callbacks run in those loop turns.
+extern thread_local int handle_cleanup_depth;
+
 class DebugSealHandleScope {
  public:
   explicit inline DebugSealHandleScope(v8::Isolate* isolate = nullptr)