Skip to content

Commit 5ff7f42

Browse files
addaleaxtargos
authored andcommitted
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 21fbcb6 commit 5ff7f42

File tree

4 files changed

+62
-3
lines changed

4 files changed

+62
-3
lines changed

src/async_wrap.cc

+2-1
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

+3-1
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

+2-1
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

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

0 commit comments

Comments
 (0)