src: fix task queue deadlock when built as C++23

Since #65353, FlushForegroundTasksInternal() and the DelayedTaskScheduler
drain their queues with `for (auto& task : queue.Lock().PopAll())`. In
C++20 the Locked temporary is destroyed at the end of the range
initializer, before the loop body runs. C++23 (P2718R0) extends the
lifetime of every temporary in a range-for initializer to the end of
the loop, so the queue's mutex stays held while the tasks run, and the
first task that posts to the same queue - any V8 foreground task that
schedules another one - deadlocks on the non-recursive mutex. Node.js
itself builds with -std=gnu++20, but an embedder that compiles it as
C++23 hangs in the first foreground task flush.

Store the drained tasks in a local before iterating so the lock is
released independent of the language version.

Refs: #65353
Signed-off-by: Shelley Vohr <shelley.vohr@gmail.com>

Author: codebytere

src/node_platform.cc

@@ -159,7 +159,9 @@ class WorkerThreadsTaskRunner::DelayedTaskScheduler {
 
     // ScheduleTasks (start a timer that pops the task into the worker queue)
     // in posting order, then, once Stop() was called, the StopTask.
-    for (std::unique_ptr<Task>& task : scheduler->tasks_.Lock().PopAll()) {
+    std::vector<std::unique_ptr<Task>> tasks =
+        scheduler->tasks_.Lock().PopAll();
+    for (std::unique_ptr<Task>& task : tasks) {
       task->Run();
     }
   }
@@ -605,8 +607,9 @@ void NodePlatform::DrainTasks(Isolate* isolate) {
 bool PerIsolatePlatformData::FlushForegroundTasksInternal() {
   bool did_work = false;
 
-  for (std::unique_ptr<DelayedTask>& delayed :
-       foreground_delayed_tasks_.Lock().PopAll()) {
+  std::vector<std::unique_ptr<DelayedTask>> delayed_tasks =
+      foreground_delayed_tasks_.Lock().PopAll();
+  for (std::unique_ptr<DelayedTask>& delayed : delayed_tasks) {
     did_work = true;
     uint64_t delay_millis = llround(delayed->timeout * 1000);
 
@@ -629,8 +632,9 @@ bool PerIsolatePlatformData::FlushForegroundTasksInternal() {
         });
   }
 
-  for (std::unique_ptr<TaskQueueEntry>& entry :
-       foreground_tasks_.Lock().PopAll()) {
+  std::vector<std::unique_ptr<TaskQueueEntry>> tasks =
+      foreground_tasks_.Lock().PopAll();
+  for (std::unique_ptr<TaskQueueEntry>& entry : tasks) {
     did_work = true;
     RunForegroundTask(std::move(entry->task));
   }

src/node_platform.h

@@ -35,7 +35,9 @@ class TaskQueue {
     void NotifyOfOutstandingCompletion();
     void BlockingDrain();
     void Stop();
-    // All queued tasks, in the order Pop() would have returned them.
+    // All queued tasks, in the order Pop() would have returned them. Store the
+    // result before iterating it: used directly as a range-for initializer,
+    // `Lock().PopAll()` keeps the lock held for the whole loop from C++23 on.
     std::vector<std::unique_ptr<T>> PopAll();
 
    private: