From 00823f29677c2fc6d2c543970cf18769d09b6a31 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
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 <benjamingr@gmail.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
---
 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<v8::Value> {
+      return v8::Object::New(isolate_);
+    });
+
+    (*env)->SetImmediate([&](node::Environment* env_arg) {
+      EXPECT_EQ(env_arg, *env);
+      isolate_->EnqueueMicrotask([](void* arg) {
+        ++*static_cast<int*>(arg);
+      }, &called);
+    }, node::CallbackFlags::kRefed);
+    uv_run(&current_loop, UV_RUN_DEFAULT);
+  }
+
+  EXPECT_EQ(called, 1);
+}