From 0e077a55af94e9b775492a12109380ec5dd24749 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Tue, 8 Dec 2020 22:48:02 +0100 Subject: [PATCH] =?UTF-8?q?src:=20use=20correct=20outer=20Context=E2=80=99?= =?UTF-8?q?s=20microtask=20queue?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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: https://github.com/nodejs/node/pull/36482 Refs: https://github.com/v8/v8/commit/4bf051d536a172e7932845d82f918ad7280c7873 Reviewed-By: Gus Caplan Reviewed-By: James M Snell Reviewed-By: Rich Trott --- src/async_wrap.cc | 3 +- src/node_contextify.cc | 4 ++- src/node_task_queue.cc | 3 +- test/cctest/test_environment.cc | 55 +++++++++++++++++++++++++++++++++ 4 files changed, 62 insertions(+), 3 deletions(-) diff --git a/src/async_wrap.cc b/src/async_wrap.cc index d6511a1dd8b3c3..218904382c4153 100644 --- a/src/async_wrap.cc +++ b/src/async_wrap.cc @@ -847,7 +847,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(arg)); }, env); diff --git a/src/node_contextify.cc b/src/node_contextify.cc index ddfe043cc41e69..d7f7548d37b469 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -199,7 +199,9 @@ MaybeLocal 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(); // 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 index b55a2a4387d655..83198486f9dfdd 100644 --- a/src/node_task_queue.cc +++ b/src/node_task_queue.cc @@ -95,7 +95,8 @@ static void EnqueueMicrotask(const FunctionCallbackInfo& args) { CHECK(args[0]->IsFunction()); - isolate->EnqueueMicrotask(args[0].As()); + isolate->GetCurrentContext()->GetMicrotaskQueue() + ->EnqueueMicrotask(isolate, args[0].As()); } static void RunMicrotasks(const FunctionCallbackInfo& args) { diff --git a/test/cctest/test_environment.cc b/test/cctest/test_environment.cc index fd4adf17c83ac9..12231151b41233 100644 --- a/test/cctest/test_environment.cc +++ b/test/cctest/test_environment.cc @@ -649,3 +649,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 queue = v8::MicrotaskQueue::New(isolate_); + v8::Local context = v8::Context::New( + isolate_, nullptr, {}, {}, {}, queue.get()); + node::InitializeContext(context); + v8::Context::Scope context_scope(context); + + int callback_calls = 0; + v8::Local must_call = v8::Function::New( + context, + [](const v8::FunctionCallbackInfo& info) { + int* callback_calls = + static_cast(info.Data().As()->Value()); + *callback_calls |= info[0].As()->Value(); + }, + v8::External::New(isolate_, static_cast(&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); +}