From 5df969826df0f2bc9d977ccee8db5ecc84155f63 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Mon, 16 Dec 2019 16:03:50 -0500
Subject: [PATCH] src: better encapsulate native immediate list

Refactor for clarity and reusability. Make it more obvious that the
list is a FIFO queue.

Backport-PR-URL: https://github.com/nodejs/node/pull/32301
PR-URL: https://github.com/nodejs/node/pull/31386
Refs: https://github.com/openjs-foundation/summit/pull/240
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
---
 src/env-inl.h | 43 +++++++++++++++++++++++++++++++++++--------
 src/env.cc    | 15 ++++++---------
 src/env.h     | 16 ++++++++++++++--
 3 files changed, 55 insertions(+), 19 deletions(-)

diff --git a/src/env-inl.h b/src/env-inl.h
index e5873d8081994d..5ad84e8222a52f 100644
--- a/src/env-inl.h
+++ b/src/env-inl.h
@@ -732,18 +732,45 @@ inline void IsolateData::set_options(
   options_ = std::move(options);
 }
 
-template <typename Fn>
-void Environment::CreateImmediate(Fn&& cb, bool ref) {
-  auto callback = std::make_unique<NativeImmediateCallbackImpl<Fn>>(
-      std::move(cb), ref);
-  NativeImmediateCallback* prev_tail = native_immediate_callbacks_tail_;
+std::unique_ptr<Environment::NativeImmediateCallback>
+Environment::NativeImmediateQueue::Shift() {
+  std::unique_ptr<Environment::NativeImmediateCallback> ret = std::move(head_);
+  if (ret) {
+    head_ = ret->get_next();
+    if (!head_)
+      tail_ = nullptr;  // The queue is now empty.
+  }
+  return ret;
+}
+
+void Environment::NativeImmediateQueue::Push(
+    std::unique_ptr<Environment::NativeImmediateCallback> cb) {
+  NativeImmediateCallback* prev_tail = tail_;
 
-  native_immediate_callbacks_tail_ = callback.get();
+  tail_ = cb.get();
   if (prev_tail != nullptr)
-    prev_tail->set_next(std::move(callback));
+    prev_tail->set_next(std::move(cb));
   else
-    native_immediate_callbacks_head_ = std::move(callback);
+    head_ = std::move(cb);
+}
+
+void Environment::NativeImmediateQueue::ConcatMove(
+    NativeImmediateQueue&& other) {
+  size_ += other.size_;
+  if (tail_ != nullptr)
+    tail_->set_next(std::move(other.head_));
+  else
+    head_ = std::move(other.head_);
+  tail_ = other.tail_;
+  other.tail_ = nullptr;
+  other.size_ = 0;
+}
 
+template <typename Fn>
+void Environment::CreateImmediate(Fn&& cb, bool ref) {
+  auto callback = std::make_unique<NativeImmediateCallbackImpl<Fn>>(
+      std::move(cb), ref);
+  native_immediates_.Push(std::move(callback));
   immediate_info()->count_inc(1);
 }
 
diff --git a/src/env.cc b/src/env.cc
index 68199f23c7a200..61832920347c6b 100644
--- a/src/env.cc
+++ b/src/env.cc
@@ -665,14 +665,14 @@ void Environment::RunAndClearNativeImmediates(bool only_refed) {
                               "RunAndClearNativeImmediates", this);
   size_t ref_count = 0;
   size_t count = 0;
-  std::unique_ptr<NativeImmediateCallback> head;
-  head.swap(native_immediate_callbacks_head_);
-  native_immediate_callbacks_tail_ = nullptr;
+
+  NativeImmediateQueue queue;
+  queue.ConcatMove(std::move(native_immediates_));
 
   auto drain_list = [&]() {
     TryCatchScope try_catch(this);
-    for (; head; head = head->get_next()) {
-      DebugSealHandleScope seal_handle_scope(isolate());
+    DebugSealHandleScope seal_handle_scope(isolate());
+    while (std::unique_ptr<NativeImmediateCallback> head = queue.Shift()) {
       count++;
       if (head->is_refed())
         ref_count++;
@@ -684,15 +684,12 @@ void Environment::RunAndClearNativeImmediates(bool only_refed) {
         if (!try_catch.HasTerminated() && can_call_into_js())
           errors::TriggerUncaughtException(isolate(), try_catch);
 
-        // We are done with the current callback. Move one iteration along,
-        // as if we had completed successfully.
-        head = head->get_next();
         return true;
       }
     }
     return false;
   };
-  while (head && drain_list()) {}
+  while (queue.size() > 0 && drain_list()) {}
 
   DCHECK_GE(immediate_info()->count(), count);
   immediate_info()->count_dec(count);
diff --git a/src/env.h b/src/env.h
index 7591c7ebfd1ec0..ce4668b5839968 100644
--- a/src/env.h
+++ b/src/env.h
@@ -1425,8 +1425,20 @@ class Environment : public MemoryRetainer {
     Fn callback_;
   };
 
-  std::unique_ptr<NativeImmediateCallback> native_immediate_callbacks_head_;
-  NativeImmediateCallback* native_immediate_callbacks_tail_ = nullptr;
+  class NativeImmediateQueue {
+   public:
+    inline std::unique_ptr<NativeImmediateCallback> Shift();
+    inline void Push(std::unique_ptr<NativeImmediateCallback> cb);
+    // ConcatMove adds elements from 'other' to the end of this list, and clears
+    // 'other' afterwards.
+    inline void ConcatMove(NativeImmediateQueue&& other);
+
+   private:
+    std::unique_ptr<NativeImmediateCallback> head_;
+    NativeImmediateCallback* tail_ = nullptr;
+  };
+
+  NativeImmediateQueue native_immediates_;
 
   void RunAndClearNativeImmediates(bool only_refed = false);
   static void CheckImmediate(uv_check_t* handle);