Skip to content

Commit

Permalink
test: regression tests for async_hooks + Promise + Worker interaction
Browse files Browse the repository at this point in the history
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 13c5a16, although they
should have the same behavior as the -3 and -4 tests).

PR-URL: #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>
  • Loading branch information
addaleax authored and codebytere committed Jun 9, 2020
1 parent ba93c8d commit 5da7d52
Show file tree
Hide file tree
Showing 4 changed files with 80 additions and 0 deletions.
15 changes: 15 additions & 0 deletions 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());
21 changes: 21 additions & 0 deletions 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());
20 changes: 20 additions & 0 deletions 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());
24 changes: 24 additions & 0 deletions 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)));

0 comments on commit 5da7d52

Please sign in to comment.