src: use correct outer Context’s microtask queue

Fall back to using the outer context’s microtask queue, rather than the Isolate’s default one. This would otherwise result in surprising behavior if an embedder specified a custom microtask queue for the main Node.js context. PR-URL: nodejs#36482 Refs: v8/v8@4bf051d Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Rich Trott <[email protected]>
addaleax · Dec 13, 2020 · c6c8337 · c6c8337
1 parent a91a95f
commit c6c8337
Show file tree

Hide file tree

Showing 4 changed files with 62 additions and 3 deletions.
diff --git a/src/async_wrap.cc b/src/async_wrap.cc
@@ -827,7 +827,8 @@ void AsyncWrap::EmitDestroy(Environment* env, double async_id) {
   // interrupt to get this Microtask scheduled as fast as possible.
   if (env->destroy_async_id_list()->size() == 16384) {
     env->RequestInterrupt([](Environment* env) {
-      env->isolate()->EnqueueMicrotask(
+      env->context()->GetMicrotaskQueue()->EnqueueMicrotask(
+        env->isolate(),
         [](void* arg) {
           DestroyAsyncIdsCallback(static_cast<Environment*>(arg));
         }, env);

diff --git a/src/node_contextify.cc b/src/node_contextify.cc
@@ -199,7 +199,9 @@ MaybeLocal<Context> ContextifyContext::CreateV8Context(
       object_template,
       {},       // global object
       {},       // deserialization callback
-      microtask_queue() ? microtask_queue().get() : nullptr);
+      microtask_queue() ?
+          microtask_queue().get() :
+          env->isolate()->GetCurrentContext()->GetMicrotaskQueue());
   if (ctx.IsEmpty()) return MaybeLocal<Context>();
   // Only partially initialize the context - the primordials are left out
   // and only initialized when necessary.

diff --git a/src/node_task_queue.cc b/src/node_task_queue.cc
@@ -96,7 +96,8 @@ static void EnqueueMicrotask(const FunctionCallbackInfo<Value>& args) {
 
   CHECK(args[0]->IsFunction());
 
-  isolate->EnqueueMicrotask(args[0].As<Function>());
+  isolate->GetCurrentContext()->GetMicrotaskQueue()
+      ->EnqueueMicrotask(isolate, args[0].As<Function>());
 }
 
 static void RunMicrotasks(const FunctionCallbackInfo<Value>& args) {

diff --git a/test/cctest/test_environment.cc b/test/cctest/test_environment.cc
@@ -611,3 +611,58 @@ TEST_F(NodeZeroIsolateTestFixture, CtrlCWithOnlySafeTerminationTest) {
   isolate->Dispose();
 }
 #endif  // _WIN32
+
+TEST_F(EnvironmentTest, NestedMicrotaskQueue) {
+  const v8::HandleScope handle_scope(isolate_);
+  const Argv argv;
+
+  std::unique_ptr<v8::MicrotaskQueue> queue = v8::MicrotaskQueue::New(isolate_);
+  v8::Local<v8::Context> context = v8::Context::New(
+      isolate_, nullptr, {}, {}, {}, queue.get());
+  node::InitializeContext(context);
+  v8::Context::Scope context_scope(context);
+
+  int callback_calls = 0;
+  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();
+      },
+      v8::External::New(isolate_, static_cast<void*>(&callback_calls)))
+          .ToLocalChecked();
+  context->Global()->Set(
+      context,
+      v8::String::NewFromUtf8Literal(isolate_, "mustCall"),
+      must_call).Check();
+
+  node::IsolateData* isolate_data = node::CreateIsolateData(
+      isolate_, &NodeTestFixture::current_loop, platform.get());
+  CHECK_NE(nullptr, isolate_data);
+
+  node::Environment* env = node::CreateEnvironment(
+      isolate_data, context, {}, {});
+  CHECK_NE(nullptr, env);
+
+  node::LoadEnvironment(
+      env,
+      "Promise.resolve().then(() => mustCall(1 << 0));\n"
+      "require('vm').runInNewContext("
+      "    'Promise.resolve().then(() => mustCall(1 << 1))',"
+      "    { mustCall },"
+      "    { microtaskMode: 'afterEvaluate' }"
+      ");"
+      "require('vm').runInNewContext("
+      "    'Promise.resolve().then(() => mustCall(1 << 2))',"
+      "    { mustCall }"
+      ");").ToLocalChecked();
+  EXPECT_EQ(callback_calls, 1 << 1);
+  isolate_->PerformMicrotaskCheckpoint();
+  EXPECT_EQ(callback_calls, 1 << 1);
+  queue->PerformCheckpoint(isolate_);
+  EXPECT_EQ(callback_calls, (1 << 0) | (1 << 1) | (1 << 2));
+
+  node::FreeEnvironment(env);
+  node::FreeIsolateData(isolate_data);
+}