async_hooks: execute destroy hooks earlier

Flarna · addaleax · commit 95e0f8ef5231 · 2020-09-22T19:54:55.000+02:00
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: #34328 refs: #33896 PR-URL: #34342 Fixes: #34328 Refs: #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>
diff --git 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
@@ -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()));
