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 committed Dec 13, 2020
1 parent a91a95f commit c6c8337
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
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
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
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
Expand Up @@ -611,3 +611,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 c6c8337

Please sign in to comment.