Skip to content

Commit

Permalink
src: use correct outer Context’s microtask queue
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
addaleax authored and targos committed Dec 21, 2020
1 parent 21fbcb6 commit 5ff7f42
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 3 deletions.
3 changes: 2 additions & 1 deletion src/async_wrap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
4 changes: 3 additions & 1 deletion src/node_contextify.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
3 changes: 2 additions & 1 deletion src/node_task_queue.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
55 changes: 55 additions & 0 deletions test/cctest/test_environment.cc
Original file line number Diff line number Diff line change
Expand Up @@ -618,3 +618,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);
}

0 comments on commit 5ff7f42

Please sign in to comment.