From 1d531f4e32d326d4fdfc94e417ffab35609b570d Mon Sep 17 00:00:00 2001
From: Gerhard Stoebich <18708370+Flarna@users.noreply.github.com>
Date: Mon, 13 Jul 2020 20:31:00 +0200
Subject: [PATCH 1/2] async_hooks: execute destroy hooks faster

Use a microtask to call destroy hooks in case there are a lot queued
as immediate may be scheduled late in case of long running
promise chains.

Queuing a mircrotasks in GC context is not allowed therefore an
interrupt is triggered to do this in JS context as fast as possible.

fixes: https://github.com/nodejs/node/issues/34328
refs: https://github.com/nodejs/node/issues/33896
---
 src/async_wrap.cc                            | 11 +++
 test/async-hooks/test-destroy-not-blocked.js | 97 ++++++++++++++++++++
 2 files changed, 108 insertions(+)
 create mode 100644 test/async-hooks/test-destroy-not-blocked.js

diff --git a/src/async_wrap.cc b/src/async_wrap.cc
index 26451f22b51a34..473aff51e98b89 100644
--- a/src/async_wrap.cc
+++ b/src/async_wrap.cc
@@ -856,6 +856,17 @@ void AsyncWrap::EmitDestroy(Environment* env, double async_id) {
     env->SetImmediate(&DestroyAsyncIdsCallback, CallbackFlags::kUnrefed);
   }
 
+  // If the list gets very large empty it faster using a Microtask.
+  // Microtasks can't be added in GC context therefore we use an
+  // 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(
+        reinterpret_cast<v8::MicrotaskCallback>(DestroyAsyncIdsCallback),
+        env);
+      });
+  }
+
   env->destroy_async_id_list()->push_back(async_id);
 }
 
diff --git a/test/async-hooks/test-destroy-not-blocked.js b/test/async-hooks/test-destroy-not-blocked.js
new file mode 100644
index 00000000000000..aa467f30143806
--- /dev/null
+++ b/test/async-hooks/test-destroy-not-blocked.js
@@ -0,0 +1,97 @@
+'use strict';
+// Flags: --expose_gc
+
+const common = require('../common');
+const assert = require('assert');
+const tick = require('../common/tick');
+
+const { createHook, AsyncResource } = require('async_hooks');
+
+// Test priority of destroy hook relative to nextTick,... and
+// verify a microtask is scheduled in case a lot items are queued
+
+const resType = 'MyResource';
+let activeId = -1;
+createHook({
+  init(id, type) {
+    if (type === resType) {
+      assert.strictEqual(activeId, -1);
+      activeId = id;
+    }
+  },
+  destroy(id) {
+    if (activeId === id) {
+      activeId = -1;
+    }
+  }
+}).enable();
+
+function testNextTick() {
+  assert.strictEqual(activeId, -1);
+  const res = new AsyncResource(resType);
+  assert.strictEqual(activeId, res.asyncId());
+  res.emitDestroy();
+  // nextTick has higher prio than emit destroy
+  process.nextTick(common.mustCall(() =>
+    assert.strictEqual(activeId, res.asyncId()))
+  );
+}
+
+function testQueueMicrotask() {
+  assert.strictEqual(activeId, -1);
+  const res = new AsyncResource(resType);
+  assert.strictEqual(activeId, res.asyncId());
+  res.emitDestroy();
+  // queueMicrotask has higher prio than emit destroy
+  queueMicrotask(common.mustCall(() =>
+    assert.strictEqual(activeId, res.asyncId()))
+  );
+}
+
+function testImmediate() {
+  assert.strictEqual(activeId, -1);
+  const res = new AsyncResource(resType);
+  assert.strictEqual(activeId, res.asyncId());
+  res.emitDestroy();
+  setImmediate(common.mustCall(() =>
+    assert.strictEqual(activeId, -1))
+  );
+}
+
+function testPromise() {
+  assert.strictEqual(activeId, -1);
+  const res = new AsyncResource(resType);
+  assert.strictEqual(activeId, res.asyncId());
+  res.emitDestroy();
+  // Promise has higher prio than emit destroy
+  Promise.resolve().then(common.mustCall(() =>
+    assert.strictEqual(activeId, res.asyncId()))
+  );
+}
+
+async function testAwait() {
+  assert.strictEqual(activeId, -1);
+  const res = new AsyncResource(resType);
+  assert.strictEqual(activeId, res.asyncId());
+  res.emitDestroy();
+
+  for (let i = 0; i < 5000; i++) {
+    await Promise.resolve();
+  }
+  global.gc();
+  await Promise.resolve();
+  // Limit to trigger a microtask not yet reached
+  assert.strictEqual(activeId, res.asyncId());
+  for (let i = 0; i < 5000; i++) {
+    await Promise.resolve();
+  }
+  global.gc();
+  await Promise.resolve();
+  assert.strictEqual(activeId, -1);
+}
+
+testNextTick();
+tick(2, testQueueMicrotask);
+tick(4, testImmediate);
+tick(6, testPromise);
+tick(8, () => testAwait().then(common.mustCall()));

From 9a6b5a6ed95c20df42c44288a359e0b496cac727 Mon Sep 17 00:00:00 2001
From: Gerhard Stoebich <18708370+Flarna@users.noreply.github.com>
Date: Fri, 31 Jul 2020 01:46:55 +0200
Subject: [PATCH 2/2] improve readability

---
 src/async_wrap.cc | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/src/async_wrap.cc b/src/async_wrap.cc
index 473aff51e98b89..05c63cbc2db38d 100644
--- a/src/async_wrap.cc
+++ b/src/async_wrap.cc
@@ -862,8 +862,9 @@ void AsyncWrap::EmitDestroy(Environment* env, double async_id) {
   if (env->destroy_async_id_list()->size() == 16384) {
     env->RequestInterrupt([](Environment* env) {
       env->isolate()->EnqueueMicrotask(
-        reinterpret_cast<v8::MicrotaskCallback>(DestroyAsyncIdsCallback),
-        env);
+        [](void* arg) {
+          DestroyAsyncIdsCallback(static_cast<Environment*>(arg));
+        }, env);
       });
   }