From 00823f29677c2fc6d2c543970cf18769d09b6a31 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Tue, 14 Jul 2020 19:06:37 +0200 Subject: [PATCH] src: add callback scope for native immediates MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This ensures that microtasks scheduled by native immediates are run after the tasks are done. In particular, this affects the inspector integration since 6f9f546406820dc. Fixes: https://github.com/nodejs/node/issues/33002 Refs: https://github.com/nodejs/node/pull/32523 PR-URL: https://github.com/nodejs/node/pull/34366 Reviewed-By: Benjamin Gruenbaum Reviewed-By: Gerhard Stöbich Reviewed-By: James M Snell --- src/env.cc | 3 +++ test/async-hooks/test-late-hook-enable.js | 16 ++++++++------ test/cctest/test_environment.cc | 26 +++++++++++++++++++++++ 3 files changed, 38 insertions(+), 7 deletions(-) diff --git a/src/env.cc b/src/env.cc index 1cf37d4fa91c8c..62012262aa01fe 100644 --- a/src/env.cc +++ b/src/env.cc @@ -698,6 +698,9 @@ void Environment::RunAndClearInterrupts() { void Environment::RunAndClearNativeImmediates(bool only_refed) { TraceEventScope trace_scope(TRACING_CATEGORY_NODE1(environment), "RunAndClearNativeImmediates", this); + HandleScope handle_scope(isolate_); + InternalCallbackScope cb_scope(this, Object::New(isolate_), { 0, 0 }); + size_t ref_count = 0; // Handle interrupts first. These functions are not allowed to throw diff --git a/test/async-hooks/test-late-hook-enable.js b/test/async-hooks/test-late-hook-enable.js index 0ffa6b24a3077d..82a80b24bb777c 100644 --- a/test/async-hooks/test-late-hook-enable.js +++ b/test/async-hooks/test-late-hook-enable.js @@ -17,14 +17,16 @@ const fnsToTest = [setTimeout, (cb) => { }); }); }, (cb) => { - process.nextTick(() => { - cb(); + setImmediate(() => { + process.nextTick(() => { + cb(); - // We need to keep the event loop open for this to actually work - // since destroy hooks are triggered in unrefed Immediates - setImmediate(() => { - hook.disable(); - assert.strictEqual(fnsToTest.length, 0); + // We need to keep the event loop open for this to actually work + // since destroy hooks are triggered in unrefed Immediates + setImmediate(() => { + hook.disable(); + assert.strictEqual(fnsToTest.length, 0); + }); }); }); }]; diff --git a/test/cctest/test_environment.cc b/test/cctest/test_environment.cc index 6de332b59b2fe5..27706044b800f6 100644 --- a/test/cctest/test_environment.cc +++ b/test/cctest/test_environment.cc @@ -533,3 +533,29 @@ TEST_F(EnvironmentTest, ExitHandlerTest) { node::LoadEnvironment(*env, "process.exit(42)").ToLocalChecked(); EXPECT_EQ(callback_calls, 1); } + +TEST_F(EnvironmentTest, SetImmediateMicrotasks) { + int called = 0; + + { + const v8::HandleScope handle_scope(isolate_); + const Argv argv; + Env env {handle_scope, argv}; + + node::LoadEnvironment(*env, + [&](const node::StartExecutionCallbackInfo& info) + -> v8::MaybeLocal { + return v8::Object::New(isolate_); + }); + + (*env)->SetImmediate([&](node::Environment* env_arg) { + EXPECT_EQ(env_arg, *env); + isolate_->EnqueueMicrotask([](void* arg) { + ++*static_cast(arg); + }, &called); + }, node::CallbackFlags::kRefed); + uv_run(¤t_loop, UV_RUN_DEFAULT); + } + + EXPECT_EQ(called, 1); +}