Skip to content

Commit

Permalink
src: use correct microtask queue for checkpoints
Browse files Browse the repository at this point in the history
I missed in c6c8337 that we should not just use that queue for
enqueuing microtasks, but also for running them.

Refs: nodejs#36482

PR-URL: nodejs#36581
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
  • Loading branch information
addaleax committed May 23, 2021
1 parent 0e077a5 commit df3d763
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 17 deletions.
3 changes: 1 addition & 2 deletions src/api/callback.cc
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ using v8::HandleScope;
using v8::Isolate;
using v8::Local;
using v8::MaybeLocal;
using v8::MicrotasksScope;
using v8::Object;
using v8::String;
using v8::Value;
Expand Down Expand Up @@ -115,7 +114,7 @@ void InternalCallbackScope::Close() {
auto weakref_cleanup = OnScopeLeave([&]() { env_->RunWeakRefCleanup(); });

if (!tick_info->has_tick_scheduled()) {
MicrotasksScope::PerformCheckpoint(env_->isolate());
env_->context()->GetMicrotaskQueue()->PerformCheckpoint(env_->isolate());

perform_stopping_check();
}
Expand Down
4 changes: 2 additions & 2 deletions src/node_task_queue.cc
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ using v8::kPromiseRejectAfterResolved;
using v8::kPromiseRejectWithNoHandler;
using v8::kPromiseResolveAfterResolved;
using v8::Local;
using v8::MicrotasksScope;
using v8::Number;
using v8::Object;
using v8::Promise;
Expand Down Expand Up @@ -100,7 +99,8 @@ static void EnqueueMicrotask(const FunctionCallbackInfo<Value>& args) {
}

static void RunMicrotasks(const FunctionCallbackInfo<Value>& args) {
MicrotasksScope::PerformCheckpoint(args.GetIsolate());
Environment* env = Environment::GetCurrent(args);
env->context()->GetMicrotaskQueue()->PerformCheckpoint(env->isolate());
}

static void SetTickCallback(const FunctionCallbackInfo<Value>& args) {
Expand Down
44 changes: 31 additions & 13 deletions test/cctest/test_environment.cc
Original file line number Diff line number Diff line change
Expand Up @@ -660,13 +660,14 @@ TEST_F(EnvironmentTest, NestedMicrotaskQueue) {
node::InitializeContext(context);
v8::Context::Scope context_scope(context);

int callback_calls = 0;
using IntVec = std::vector<int>;
IntVec callback_calls;
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();
IntVec* callback_calls = static_cast<IntVec*>(
info.Data().As<v8::External>()->Value());
callback_calls->push_back(info[0].As<v8::Int32>()->Value());
},
v8::External::New(isolate_, static_cast<void*>(&callback_calls)))
.ToLocalChecked();
Expand All @@ -683,23 +684,40 @@ TEST_F(EnvironmentTest, NestedMicrotaskQueue) {
isolate_data, context, {}, {});
CHECK_NE(nullptr, env);

node::LoadEnvironment(
v8::Local<v8::Function> eval_in_env = node::LoadEnvironment(
env,
"Promise.resolve().then(() => mustCall(1 << 0));\n"
"mustCall(1);\n"
"Promise.resolve().then(() => mustCall(2));\n"
"require('vm').runInNewContext("
" 'Promise.resolve().then(() => mustCall(1 << 1))',"
" 'Promise.resolve().then(() => mustCall(3))',"
" { mustCall },"
" { microtaskMode: 'afterEvaluate' }"
");"
");\n"
"require('vm').runInNewContext("
" 'Promise.resolve().then(() => mustCall(1 << 2))',"
" 'Promise.resolve().then(() => mustCall(4))',"
" { mustCall }"
");").ToLocalChecked();
EXPECT_EQ(callback_calls, 1 << 1);
");\n"
"setTimeout(() => {"
" Promise.resolve().then(() => mustCall(5));"
"}, 10);\n"
"mustCall(6);\n"
"return eval;\n").ToLocalChecked().As<v8::Function>();
EXPECT_EQ(callback_calls, (IntVec { 1, 3, 6, 2, 4 }));
v8::Local<v8::Value> queue_microtask_code = v8::String::NewFromUtf8Literal(
isolate_, "queueMicrotask(() => mustCall(7));");
eval_in_env->Call(context,
v8::Null(isolate_),
1,
&queue_microtask_code).ToLocalChecked();
EXPECT_EQ(callback_calls, (IntVec { 1, 3, 6, 2, 4 }));
isolate_->PerformMicrotaskCheckpoint();
EXPECT_EQ(callback_calls, 1 << 1);
EXPECT_EQ(callback_calls, (IntVec { 1, 3, 6, 2, 4 }));
queue->PerformCheckpoint(isolate_);
EXPECT_EQ(callback_calls, (1 << 0) | (1 << 1) | (1 << 2));
EXPECT_EQ(callback_calls, (IntVec { 1, 3, 6, 2, 4, 7 }));

int exit_code = SpinEventLoop(env).FromJust();
EXPECT_EQ(exit_code, 0);
EXPECT_EQ(callback_calls, (IntVec { 1, 3, 6, 2, 4, 7, 5 }));

node::FreeEnvironment(env);
node::FreeIsolateData(isolate_data);
Expand Down

0 comments on commit df3d763

Please sign in to comment.