Skip to content

Commit 0e077a5

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: nodejs#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 c46dd77 commit 0e077a5

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
@@ -847,7 +847,8 @@ void AsyncWrap::EmitDestroy(Environment* env, double async_id) {
847847
// interrupt to get this Microtask scheduled as fast as possible.
848848
if (env->destroy_async_id_list()->size() == 16384) {
849849
env->RequestInterrupt([](Environment* env) {
850-
env->isolate()->EnqueueMicrotask(
850+
env->context()->GetMicrotaskQueue()->EnqueueMicrotask(
851+
env->isolate(),
851852
[](void* arg) {
852853
DestroyAsyncIdsCallback(static_cast<Environment*>(arg));
853854
}, 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
@@ -95,7 +95,8 @@ static void EnqueueMicrotask(const FunctionCallbackInfo<Value>& args) {
9595

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

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

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

test/cctest/test_environment.cc

+55
Original file line numberDiff line numberDiff line change
@@ -649,3 +649,58 @@ TEST_F(NodeZeroIsolateTestFixture, CtrlCWithOnlySafeTerminationTest) {
649649
isolate->Dispose();
650650
}
651651
#endif // _WIN32
652+
653+
TEST_F(EnvironmentTest, NestedMicrotaskQueue) {
654+
const v8::HandleScope handle_scope(isolate_);
655+
const Argv argv;
656+
657+
std::unique_ptr<v8::MicrotaskQueue> queue = v8::MicrotaskQueue::New(isolate_);
658+
v8::Local<v8::Context> context = v8::Context::New(
659+
isolate_, nullptr, {}, {}, {}, queue.get());
660+
node::InitializeContext(context);
661+
v8::Context::Scope context_scope(context);
662+
663+
int callback_calls = 0;
664+
v8::Local<v8::Function> must_call = v8::Function::New(
665+
context,
666+
[](const v8::FunctionCallbackInfo<v8::Value>& info) {
667+
int* callback_calls =
668+
static_cast<int*>(info.Data().As<v8::External>()->Value());
669+
*callback_calls |= info[0].As<v8::Int32>()->Value();
670+
},
671+
v8::External::New(isolate_, static_cast<void*>(&callback_calls)))
672+
.ToLocalChecked();
673+
context->Global()->Set(
674+
context,
675+
v8::String::NewFromUtf8Literal(isolate_, "mustCall"),
676+
must_call).Check();
677+
678+
node::IsolateData* isolate_data = node::CreateIsolateData(
679+
isolate_, &NodeTestFixture::current_loop, platform.get());
680+
CHECK_NE(nullptr, isolate_data);
681+
682+
node::Environment* env = node::CreateEnvironment(
683+
isolate_data, context, {}, {});
684+
CHECK_NE(nullptr, env);
685+
686+
node::LoadEnvironment(
687+
env,
688+
"Promise.resolve().then(() => mustCall(1 << 0));\n"
689+
"require('vm').runInNewContext("
690+
" 'Promise.resolve().then(() => mustCall(1 << 1))',"
691+
" { mustCall },"
692+
" { microtaskMode: 'afterEvaluate' }"
693+
");"
694+
"require('vm').runInNewContext("
695+
" 'Promise.resolve().then(() => mustCall(1 << 2))',"
696+
" { mustCall }"
697+
");").ToLocalChecked();
698+
EXPECT_EQ(callback_calls, 1 << 1);
699+
isolate_->PerformMicrotaskCheckpoint();
700+
EXPECT_EQ(callback_calls, 1 << 1);
701+
queue->PerformCheckpoint(isolate_);
702+
EXPECT_EQ(callback_calls, (1 << 0) | (1 << 1) | (1 << 2));
703+
704+
node::FreeEnvironment(env);
705+
node::FreeIsolateData(isolate_data);
706+
}

0 commit comments

Comments
 (0)