src: fix hang when joining workers in process.exit()

V8 background jobs, such as concurrent Sparkplug and Maglev compiles,
can run into the heap limit. They then post a GC request to the main
thread and park in CollectionBarrier::AwaitCollectionBackground()
until the main thread collects garbage or the isolate is torn down.

process.exit() does neither. DefaultProcessExitHandlerInternal()
calls DisposePlatform() while the isolate is still alive and not
running JS, and NodePlatform::Shutdown() joins the worker threads.
A worker that is parked at that point never wakes up, and the join
hangs forever.

Pass the exiting isolate down to NodePlatform::Shutdown(). While the
workers finish their current tasks, the main thread checks every
10ms whether a foreground task was posted for that isolate. If so,
it drops the queued tasks, which would never run on this path, and
performs a GC so that the parked workers can finish. Normal exits
dispose the isolate before the platform and are not affected.

Fixes: #64274
Signed-off-by: Manzoor Wani <manzoorwani.jk@gmail.com>

Author: manzoorwanijk

src/api/environment.cc

@@ -1179,7 +1179,8 @@ void DefaultProcessExitHandlerInternal(Environment* env, ExitCode exit_code) {
   // And make sure V8Platform don not call into Libuv threadpool, see Dispose
   // in node_v8_platform-inl.h
   uv_library_shutdown();
-  DisposePlatform();
+  // The isolate is not disposed on this path, see NodePlatform::Shutdown().
+  DisposePlatform(env->isolate());
 
 #if HAVE_OPENSSL
   crypto::CleanupCachedRootCertificates();

src/node_platform.cc

@@ -22,6 +22,9 @@ struct PlatformWorkerData {
   Mutex* platform_workers_mutex;
   ConditionVariable* platform_workers_ready;
   int* pending_platform_workers;
+  Mutex* running_workers_mutex;
+  ConditionVariable* worker_exited;
+  int* running_workers;
   int id;
   PlatformDebugLogLevel debug_log_level;
 };
@@ -83,6 +86,11 @@ static void PlatformWorkerThread(void* data) {
       pending_worker_tasks->Lock().NotifyOfOutstandingCompletion();
     }
   }
+
+  // See WorkerThreadsTaskRunner::Shutdown().
+  Mutex::ScopedLock lock(*worker_data->running_workers_mutex);
+  (*worker_data->running_workers)--;
+  worker_data->worker_exited->Signal(lock);
 }
 
 static int GetActualThreadPoolSize(int thread_pool_size) {
@@ -261,6 +269,9 @@ WorkerThreadsTaskRunner::WorkerThreadsTaskRunner(
                            &platform_workers_mutex,
                            &platform_workers_ready,
                            &pending_platform_workers,
+                           &running_workers_mutex_,
+                           &worker_exited_,
+                           &running_workers_,
                            i,
                            debug_log_level_});
     std::unique_ptr<uv_thread_t> t { new uv_thread_t() };
@@ -270,6 +281,8 @@ WorkerThreadsTaskRunner::WorkerThreadsTaskRunner(
     }
     worker_data.release();
     threads_.push_back(std::move(t));
+    Mutex::ScopedLock running_lock(running_workers_mutex_);
+    running_workers_++;
   }
 
   // Wait for platform workers to initialize before continuing with the
@@ -300,9 +313,22 @@ void WorkerThreadsTaskRunner::BlockingDrain() {
   pending_worker_tasks_.Lock().BlockingDrain();
 }
 
-void WorkerThreadsTaskRunner::Shutdown() {
+void WorkerThreadsTaskRunner::Shutdown(
+    const std::function<void()>& on_stalled) {
   pending_worker_tasks_.Lock().Stop();
   delayed_task_scheduler_->Stop();
+  if (on_stalled) {
+    /* Workers take no new tasks after Stop(), so this only waits for the
+     * tasks that are already running. */
+    constexpr uint64_t kStalledTimeoutNs = 10 * 1000 * 1000;
+    Mutex::ScopedLock lock(running_workers_mutex_);
+    while (running_workers_ > 0) {
+      if (worker_exited_.TimedWait(lock, kStalledTimeoutNs) == UV_ETIMEDOUT) {
+        Mutex::ScopedUnlock unlock(lock);
+        on_stalled();
+      }
+    }
+  }
   for (size_t i = 0; i < threads_.size(); i++) {
     CHECK_EQ(0, uv_thread_join(threads_[i].get()));
   }
@@ -523,10 +549,27 @@ void NodePlatform::AddIsolateFinishedCallback(Isolate* isolate,
   it->second.second->AddShutdownCallback(cb, data);
 }
 
-void NodePlatform::Shutdown() {
+void NodePlatform::Shutdown(Isolate* isolate) {
   if (has_shut_down_) return;
   has_shut_down_ = true;
-  worker_thread_task_runner_->Shutdown();
+  std::shared_ptr<PerIsolatePlatformData> per_isolate;
+  if (isolate != nullptr) {
+    Mutex::ScopedLock lock(per_isolate_mutex_);
+    auto it = per_isolate_.find(isolate);
+    if (it != per_isolate_.end()) per_isolate = it->second.second;
+  }
+  if (per_isolate) {
+    worker_thread_task_runner_->Shutdown([&]() {
+      /* V8 posts a foreground task when a worker has to wait for a GC, but
+       * the isolate will not run its tasks or be torn down anymore, so do the
+       * GC here. Discarding the tasks lets the next request be told apart. */
+      if (per_isolate->DiscardForegroundTasks()) {
+        isolate->LowMemoryNotification();
+      }
+    });
+  } else {
+    worker_thread_task_runner_->Shutdown();
+  }
 
   {
     Mutex::ScopedLock lock(per_isolate_mutex_);
@@ -604,6 +647,13 @@ void NodePlatform::DrainTasks(Isolate* isolate) {
   } while (per_isolate->FlushForegroundTasksInternal());
 }
 
+bool PerIsolatePlatformData::DiscardForegroundTasks() {
+  // Destroy the tasks after releasing the queue lock.
+  std::vector<std::unique_ptr<TaskQueueEntry>> tasks =
+      foreground_tasks_.Lock().PopAll();
+  return !tasks.empty();
+}
+
 bool PerIsolatePlatformData::FlushForegroundTasksInternal() {
   bool did_work = false;
 

src/node_platform.h

@@ -134,6 +134,8 @@ class PerIsolatePlatformData
   // posted during flushing of the queue are postponed until the next
   // flushing.
   bool FlushForegroundTasksInternal();
+  // Drops the queued foreground tasks. Returns true if there were any.
+  bool DiscardForegroundTasks();
 
   const uv_loop_t* event_loop() const { return loop_; }
 
@@ -202,7 +204,9 @@ class WorkerThreadsTaskRunner {
                        double delay_in_seconds);
 
   void BlockingDrain();
-  void Shutdown();
+  /* `on_stalled` runs on the calling thread whenever the workers have not
+   * finished their current tasks for a while. */
+  void Shutdown(const std::function<void()>& on_stalled = nullptr);
 
   int NumberOfWorkerThreads() const;
 
@@ -221,6 +225,10 @@ class WorkerThreadsTaskRunner {
   std::unique_ptr<DelayedTaskScheduler> delayed_task_scheduler_;
 
   std::vector<std::unique_ptr<uv_thread_t>> threads_;
+  // Number of platform worker threads that have not exited yet.
+  Mutex running_workers_mutex_;
+  ConditionVariable worker_exited_;
+  int running_workers_ = 0;
   PlatformDebugLogLevel debug_log_level_ = PlatformDebugLogLevel::kNone;
 };
 
@@ -232,7 +240,9 @@ class NodePlatform : public MultiIsolatePlatform {
   ~NodePlatform() override;
 
   void DrainTasks(v8::Isolate* isolate) override;
-  void Shutdown();
+  /* Pass the isolate that the calling thread is running when it will not be
+   * disposed first, so that tasks waiting for it to collect garbage can end. */
+  void Shutdown(v8::Isolate* isolate = nullptr);
 
   // v8::Platform implementation.
   int NumberOfWorkerThreads() override;

src/node_v8_platform-inl.h

@@ -32,12 +32,12 @@ struct V8Platform {
   }
   // Make sure V8Platform don not call into Libuv threadpool,
   // see DefaultProcessExitHandlerInternal in environment.cc
-  inline void Dispose() {
+  inline void Dispose(v8::Isolate* isolate = nullptr) {
     if (!initialized_)
       return;
     initialized_ = false;
     StopTracingAgent();
-    platform_->Shutdown();
+    platform_->Shutdown(isolate);
     platform_.reset();
     // Destroy tracing after the platform (and platform threads) have been
     // stopped.
@@ -67,7 +67,7 @@ struct V8Platform {
   std::unique_ptr<NodePlatform> platform_;
 #else   // !NODE_USE_V8_PLATFORM
   inline void Initialize(int thread_pool_size) {}
-  inline void Dispose() {}
+  inline void Dispose(v8::Isolate* isolate = nullptr) {}
   inline void DrainVMTasks(v8::Isolate* isolate) {}
   inline void StartTracingAgent() {
     if (!per_process::cli_options->trace_event_categories.empty()) {
@@ -86,8 +86,9 @@ namespace per_process {
 extern struct V8Platform v8_platform;
 }
 
-inline void DisposePlatform() {
-  per_process::v8_platform.Dispose();
+// See NodePlatform::Shutdown() for `isolate`.
+inline void DisposePlatform(v8::Isolate* isolate = nullptr) {
+  per_process::v8_platform.Dispose(isolate);
 }
 
 }  // namespace node

test/parallel/test-process-exit-background-gc.js

@@ -0,0 +1,30 @@
+'use strict';
+const common = require('../common');
+if (!common.hasCrypto) { common.skip('missing crypto'); }
+
+// Regression test for https://github.com/nodejs/node/issues/64274.
+// process.exit() joins the V8 platform worker threads without tearing down
+// the isolate, so a background task waiting for a main thread GC hung it.
+
+const { spawnSyncAndExitWithoutError } = require('../common/child_process');
+
+if (process.argv[2] === 'child') {
+  const keep = [];
+  for (let i = 0; i < 1e5; i++) keep.push({ i });
+  setTimeout(() => {
+    // uv_library_shutdown() waits for this job, keeping the main thread out
+    // of JS while the stress task runs into the heap limit and waits for GC.
+    require('crypto').pbkdf2('a', 'b', 5e6, 64, 'sha512', () => {});
+    process.exit(0);
+  }, 300);
+  return;
+}
+
+spawnSyncAndExitWithoutError(process.execPath, [
+  '--stress-concurrent-allocation',
+  __filename,
+  'child',
+], {
+  timeout: common.platformTimeout(30_000),
+  killSignal: 'SIGKILL',
+});