From 5da7d52a9ffb9df47601849173342382fa290f2a Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Mon, 11 May 2020 00:25:49 +0200
Subject: [PATCH] test: regression tests for async_hooks + Promise + Worker
 interaction
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Add regression tests for the case in which an async_hook is enabled
inside a Worker thread and `process.exit()` is called during the
async part of an async function.

This commit includes multiple tests that seem like they should all
crash in a similar way, but interestingly don’t. In particular, it’s
surprising that the presence of a statement after `process.exit()`
in a function has an effect on the kind of crash that’s being
exhibited (V8 DCHECK vs. assertion in our own code) and the
circumstances under which it crashes (e.g. the -1 and -2 tests
can be “fixed” by reverting 13c5a1629cd025b, although they
should have the same behavior as the -3 and -4 tests).

PR-URL: https://github.com/nodejs/node/pull/33347
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
---
 ...-async-hooks-worker-asyncfn-terminate-1.js | 15 ++++++++++++
 ...-async-hooks-worker-asyncfn-terminate-2.js | 21 ++++++++++++++++
 ...-async-hooks-worker-asyncfn-terminate-3.js | 20 ++++++++++++++++
 ...-async-hooks-worker-asyncfn-terminate-4.js | 24 +++++++++++++++++++
 4 files changed, 80 insertions(+)
 create mode 100644 test/parallel/test-async-hooks-worker-asyncfn-terminate-1.js
 create mode 100644 test/parallel/test-async-hooks-worker-asyncfn-terminate-2.js
 create mode 100644 test/parallel/test-async-hooks-worker-asyncfn-terminate-3.js
 create mode 100644 test/parallel/test-async-hooks-worker-asyncfn-terminate-4.js

diff --git a/test/parallel/test-async-hooks-worker-asyncfn-terminate-1.js b/test/parallel/test-async-hooks-worker-asyncfn-terminate-1.js
new file mode 100644
index 00000000000000..eb1664591308a3
--- /dev/null
+++ b/test/parallel/test-async-hooks-worker-asyncfn-terminate-1.js
@@ -0,0 +1,15 @@
+'use strict';
+const common = require('../common');
+const { Worker } = require('worker_threads');
+
+const w = new Worker(`
+const { createHook } = require('async_hooks');
+
+setImmediate(async () => {
+  createHook({ init() {} }).enable();
+  await 0;
+  process.exit();
+});
+`, { eval: true });
+
+w.on('exit', common.mustCall());
diff --git a/test/parallel/test-async-hooks-worker-asyncfn-terminate-2.js b/test/parallel/test-async-hooks-worker-asyncfn-terminate-2.js
new file mode 100644
index 00000000000000..049264d3e8aef5
--- /dev/null
+++ b/test/parallel/test-async-hooks-worker-asyncfn-terminate-2.js
@@ -0,0 +1,21 @@
+'use strict';
+const common = require('../common');
+const { Worker } = require('worker_threads');
+
+// Like test-async-hooks-worker-promise.js but with the `await` and `createHook`
+// lines switched, because that resulted in different assertion failures
+// (one a Node.js assertion and one a V8 DCHECK) and it seems prudent to
+// cover both of those failures.
+
+const w = new Worker(`
+const { createHook } = require('async_hooks');
+
+setImmediate(async () => {
+  await 0;
+  createHook({ init() {} }).enable();
+  process.exit();
+});
+`, { eval: true });
+
+w.postMessage({});
+w.on('exit', common.mustCall());
diff --git a/test/parallel/test-async-hooks-worker-asyncfn-terminate-3.js b/test/parallel/test-async-hooks-worker-asyncfn-terminate-3.js
new file mode 100644
index 00000000000000..40c7d85835ae44
--- /dev/null
+++ b/test/parallel/test-async-hooks-worker-asyncfn-terminate-3.js
@@ -0,0 +1,20 @@
+'use strict';
+const common = require('../common');
+const { Worker } = require('worker_threads');
+
+// Like test-async-hooks-worker-promise.js but with an additional statement
+// after the `process.exit()` call, that shouldn’t really make a difference
+// but apparently does.
+
+const w = new Worker(`
+const { createHook } = require('async_hooks');
+
+setImmediate(async () => {
+  createHook({ init() {} }).enable();
+  await 0;
+  process.exit();
+  process._rawDebug('THIS SHOULD NEVER BE REACHED');
+});
+`, { eval: true });
+
+w.on('exit', common.mustCall());
diff --git a/test/parallel/test-async-hooks-worker-asyncfn-terminate-4.js b/test/parallel/test-async-hooks-worker-asyncfn-terminate-4.js
new file mode 100644
index 00000000000000..dc641471b691e0
--- /dev/null
+++ b/test/parallel/test-async-hooks-worker-asyncfn-terminate-4.js
@@ -0,0 +1,24 @@
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+const { Worker } = require('worker_threads');
+
+// Like test-async-hooks-worker-promise.js but doing a trivial counter increase
+// after process.exit(). This should not make a difference, but apparently it
+// does. This is *also* different from test-async-hooks-worker-promise-3.js,
+// in that the statement is an ArrayBuffer access rather than a full method,
+// which *also* makes a difference even though it shouldn’t.
+
+const workerData = new Int32Array(new SharedArrayBuffer(4));
+const w = new Worker(`
+const { createHook } = require('async_hooks');
+
+setImmediate(async () => {
+  createHook({ init() {} }).enable();
+  await 0;
+  process.exit();
+  workerData[0]++;
+});
+`, { eval: true, workerData });
+
+w.on('exit', common.mustCall(() => assert.strictEqual(workerData[0], 0)));