From 2070d5b52431c5fb4b35a07a998f0f9b9805e90a Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Sat, 19 Dec 2020 13:25:27 +0100
Subject: [PATCH] src: use correct microtask queue for checkpoints

I missed in c6c8337402 that we should not just use that queue for
enqueuing microtasks, but also for running them.

Refs: https://github.com/nodejs/node/pull/36482

PR-URL: https://github.com/nodejs/node/pull/36581
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
---
 src/api/callback.cc             |  3 +--
 src/node_task_queue.cc          |  4 +--
 test/cctest/test_environment.cc | 44 +++++++++++++++++++++++----------
 3 files changed, 34 insertions(+), 17 deletions(-)

diff --git a/src/api/callback.cc b/src/api/callback.cc
index 8a0b71cd3626e2..a7b23dd4924baf 100644
--- a/src/api/callback.cc
+++ b/src/api/callback.cc
@@ -12,7 +12,6 @@ using v8::HandleScope;
 using v8::Isolate;
 using v8::Local;
 using v8::MaybeLocal;
-using v8::MicrotasksScope;
 using v8::Object;
 using v8::String;
 using v8::Value;
@@ -111,7 +110,7 @@ void InternalCallbackScope::Close() {
   auto weakref_cleanup = OnScopeLeave([&]() { env_->RunWeakRefCleanup(); });
 
   if (!tick_info->has_tick_scheduled()) {
-    MicrotasksScope::PerformCheckpoint(env_->isolate());
+    env_->context()->GetMicrotaskQueue()->PerformCheckpoint(env_->isolate());
 
     perform_stopping_check();
   }
diff --git a/src/node_task_queue.cc b/src/node_task_queue.cc
index dff59d8dc77b99..0aa021fc5b27df 100644
--- a/src/node_task_queue.cc
+++ b/src/node_task_queue.cc
@@ -21,7 +21,6 @@ using v8::kPromiseRejectAfterResolved;
 using v8::kPromiseRejectWithNoHandler;
 using v8::kPromiseResolveAfterResolved;
 using v8::Local;
-using v8::MicrotasksScope;
 using v8::Number;
 using v8::Object;
 using v8::Promise;
@@ -101,7 +100,8 @@ static void EnqueueMicrotask(const FunctionCallbackInfo<Value>& args) {
 }
 
 static void RunMicrotasks(const FunctionCallbackInfo<Value>& args) {
-  MicrotasksScope::PerformCheckpoint(args.GetIsolate());
+  Environment* env = Environment::GetCurrent(args);
+  env->context()->GetMicrotaskQueue()->PerformCheckpoint(env->isolate());
 }
 
 static void SetTickCallback(const FunctionCallbackInfo<Value>& args) {
diff --git a/test/cctest/test_environment.cc b/test/cctest/test_environment.cc
index 620f43dc5e2913..eca21d1dc0c98c 100644
--- a/test/cctest/test_environment.cc
+++ b/test/cctest/test_environment.cc
@@ -622,13 +622,14 @@ TEST_F(EnvironmentTest, NestedMicrotaskQueue) {
   node::InitializeContext(context);
   v8::Context::Scope context_scope(context);
 
-  int callback_calls = 0;
+  using IntVec = std::vector<int>;
+  IntVec callback_calls;
   v8::Local<v8::Function> must_call = v8::Function::New(
       context,
       [](const v8::FunctionCallbackInfo<v8::Value>& info) {
-        int* callback_calls =
-            static_cast<int*>(info.Data().As<v8::External>()->Value());
-        *callback_calls |= info[0].As<v8::Int32>()->Value();
+        IntVec* callback_calls = static_cast<IntVec*>(
+            info.Data().As<v8::External>()->Value());
+        callback_calls->push_back(info[0].As<v8::Int32>()->Value());
       },
       v8::External::New(isolate_, static_cast<void*>(&callback_calls)))
           .ToLocalChecked();
@@ -645,23 +646,40 @@ TEST_F(EnvironmentTest, NestedMicrotaskQueue) {
       isolate_data, context, {}, {});
   CHECK_NE(nullptr, env);
 
-  node::LoadEnvironment(
+  v8::Local<v8::Function> eval_in_env = node::LoadEnvironment(
       env,
-      "Promise.resolve().then(() => mustCall(1 << 0));\n"
+      "mustCall(1);\n"
+      "Promise.resolve().then(() => mustCall(2));\n"
       "require('vm').runInNewContext("
-      "    'Promise.resolve().then(() => mustCall(1 << 1))',"
+      "    'Promise.resolve().then(() => mustCall(3))',"
       "    { mustCall },"
       "    { microtaskMode: 'afterEvaluate' }"
-      ");"
+      ");\n"
       "require('vm').runInNewContext("
-      "    'Promise.resolve().then(() => mustCall(1 << 2))',"
+      "    'Promise.resolve().then(() => mustCall(4))',"
       "    { mustCall }"
-      ");").ToLocalChecked();
-  EXPECT_EQ(callback_calls, 1 << 1);
+      ");\n"
+      "setTimeout(() => {"
+      "  Promise.resolve().then(() => mustCall(5));"
+      "}, 10);\n"
+      "mustCall(6);\n"
+      "return eval;\n").ToLocalChecked().As<v8::Function>();
+  EXPECT_EQ(callback_calls, (IntVec { 1, 3, 6, 2, 4 }));
+  v8::Local<v8::Value> queue_microtask_code = v8::String::NewFromUtf8Literal(
+      isolate_, "queueMicrotask(() => mustCall(7));");
+  eval_in_env->Call(context,
+                    v8::Null(isolate_),
+                    1,
+                    &queue_microtask_code).ToLocalChecked();
+  EXPECT_EQ(callback_calls, (IntVec { 1, 3, 6, 2, 4 }));
   isolate_->PerformMicrotaskCheckpoint();
-  EXPECT_EQ(callback_calls, 1 << 1);
+  EXPECT_EQ(callback_calls, (IntVec { 1, 3, 6, 2, 4 }));
   queue->PerformCheckpoint(isolate_);
-  EXPECT_EQ(callback_calls, (1 << 0) | (1 << 1) | (1 << 2));
+  EXPECT_EQ(callback_calls, (IntVec { 1, 3, 6, 2, 4, 7 }));
+
+  int exit_code = SpinEventLoop(env).FromJust();
+  EXPECT_EQ(exit_code, 0);
+  EXPECT_EQ(callback_calls, (IntVec { 1, 3, 6, 2, 4, 7, 5 }));
 
   node::FreeEnvironment(env);
   node::FreeIsolateData(isolate_data);