Skip to content

Commit c6c8337

Browse files
committed
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: #36482 Refs: v8/v8@4bf051d Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
1 parent a91a95f commit c6c8337

File tree

4 files changed

+62
-3
lines changed

4 files changed

+62
-3
lines changed

src/async_wrap.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -827,7 +827,8 @@ void AsyncWrap::EmitDestroy(Environment* env, double async_id) {
827827
// interrupt to get this Microtask scheduled as fast as possible.
828828
if (env->destroy_async_id_list()->size() == 16384) {
829829
env->RequestInterrupt([](Environment* env) {
830-
env->isolate()->EnqueueMicrotask(
830+
env->context()->GetMicrotaskQueue()->EnqueueMicrotask(
831+
env->isolate(),
831832
[](void* arg) {
832833
DestroyAsyncIdsCallback(static_cast<Environment*>(arg));
833834
}, env);

src/node_contextify.cc

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,9 @@ MaybeLocal<Context> ContextifyContext::CreateV8Context(
199199
object_template,
200200
{}, // global object
201201
{}, // deserialization callback
202-
microtask_queue() ? microtask_queue().get() : nullptr);
202+
microtask_queue() ?
203+
microtask_queue().get() :
204+
env->isolate()->GetCurrentContext()->GetMicrotaskQueue());
203205
if (ctx.IsEmpty()) return MaybeLocal<Context>();
204206
// Only partially initialize the context - the primordials are left out
205207
// and only initialized when necessary.

src/node_task_queue.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,8 @@ static void EnqueueMicrotask(const FunctionCallbackInfo<Value>& args) {
9696

9797
CHECK(args[0]->IsFunction());
9898

99-
isolate->EnqueueMicrotask(args[0].As<Function>());
99+
isolate->GetCurrentContext()->GetMicrotaskQueue()
100+
->EnqueueMicrotask(isolate, args[0].As<Function>());
100101
}
101102

102103
static void RunMicrotasks(const FunctionCallbackInfo<Value>& args) {

test/cctest/test_environment.cc

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -611,3 +611,58 @@ TEST_F(NodeZeroIsolateTestFixture, CtrlCWithOnlySafeTerminationTest) {
611611
isolate->Dispose();
612612
}
613613
#endif // _WIN32
614+
615+
TEST_F(EnvironmentTest, NestedMicrotaskQueue) {
616+
const v8::HandleScope handle_scope(isolate_);
617+
const Argv argv;
618+
619+
std::unique_ptr<v8::MicrotaskQueue> queue = v8::MicrotaskQueue::New(isolate_);
620+
v8::Local<v8::Context> context = v8::Context::New(
621+
isolate_, nullptr, {}, {}, {}, queue.get());
622+
node::InitializeContext(context);
623+
v8::Context::Scope context_scope(context);
624+
625+
int callback_calls = 0;
626+
v8::Local<v8::Function> must_call = v8::Function::New(
627+
context,
628+
[](const v8::FunctionCallbackInfo<v8::Value>& info) {
629+
int* callback_calls =
630+
static_cast<int*>(info.Data().As<v8::External>()->Value());
631+
*callback_calls |= info[0].As<v8::Int32>()->Value();
632+
},
633+
v8::External::New(isolate_, static_cast<void*>(&callback_calls)))
634+
.ToLocalChecked();
635+
context->Global()->Set(
636+
context,
637+
v8::String::NewFromUtf8Literal(isolate_, "mustCall"),
638+
must_call).Check();
639+
640+
node::IsolateData* isolate_data = node::CreateIsolateData(
641+
isolate_, &NodeTestFixture::current_loop, platform.get());
642+
CHECK_NE(nullptr, isolate_data);
643+
644+
node::Environment* env = node::CreateEnvironment(
645+
isolate_data, context, {}, {});
646+
CHECK_NE(nullptr, env);
647+
648+
node::LoadEnvironment(
649+
env,
650+
"Promise.resolve().then(() => mustCall(1 << 0));\n"
651+
"require('vm').runInNewContext("
652+
" 'Promise.resolve().then(() => mustCall(1 << 1))',"
653+
" { mustCall },"
654+
" { microtaskMode: 'afterEvaluate' }"
655+
");"
656+
"require('vm').runInNewContext("
657+
" 'Promise.resolve().then(() => mustCall(1 << 2))',"
658+
" { mustCall }"
659+
");").ToLocalChecked();
660+
EXPECT_EQ(callback_calls, 1 << 1);
661+
isolate_->PerformMicrotaskCheckpoint();
662+
EXPECT_EQ(callback_calls, 1 << 1);
663+
queue->PerformCheckpoint(isolate_);
664+
EXPECT_EQ(callback_calls, (1 << 0) | (1 << 1) | (1 << 2));
665+
666+
node::FreeEnvironment(env);
667+
node::FreeIsolateData(isolate_data);
668+
}

0 commit comments

Comments
 (0)