From 95e0f8ef523130c8bd03c222cc378278ceafb11d 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] async_hooks: execute destroy hooks earlier

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

PR-URL: https://github.com/nodejs/node/pull/34342
Fixes: https://github.com/nodejs/node/issues/34328
Refs: https://github.com/nodejs/node/issues/33896
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
---
 src/async_wrap.cc                            | 12 +++
 test/async-hooks/test-destroy-not-blocked.js | 97 ++++++++++++++++++++
 2 files changed, 109 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 8056e4ba8bbe81..f260afa0dd5cc4 100644
--- a/src/async_wrap.cc
+++ b/src/async_wrap.cc
@@ -688,6 +688,18 @@ void AsyncWrap::EmitDestroy(Environment* env, double async_id) {
     env->SetUnrefImmediate(&DestroyAsyncIdsCallback);
   }
 
+  // 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(
+        [](void* arg) {
+          DestroyAsyncIdsCallback(static_cast<Environment*>(arg));
+        }, 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()));