node/test/parallel/test-worker-dns-terminate.js
Anna Henningsen ed24c19002
worker: refactor worker.terminate()
At the collaborator summit in Berlin, the behaviour of
`worker.terminate()` was discussed.

In particular, switching from a callback-based to a Promise-based API
was suggested. While investigating that possibility later, it was
discovered that `.terminate()` was unintentionally synchronous up
until now (including calling its callback synchronously).

Also, the topic of its stability has been brought up. I have performed
two manual reviews of the native codebase for compatibility with
`.terminate()`, and performed some manual fuzz testing with the test
suite. At this point, bugs with `.terminate()` should, in my opinion,
be treated like bugs in other Node.js features.
(It is possible to make Node.js crash with `.terminate()` by messing
with internals and/or built-in prototype objects, but that is already
the case without `.terminate()` as well.)

This commit:

- Makes `.terminate()` an asynchronous operation.
- Makes `.terminate()` return a `Promise`.
- Runtime-deprecates passing a callback.
- Removes a warning about its stability from the documentation.
- Eliminates an unnecessary extra function from the C++ code.

A possible alternative to returning a `Promise` would be to keep the
method synchronous and just drop the callback. Generally, providing
an asynchronous API does provide us with a bit more flexibility.

Refs: https://github.com/nodejs/summit/issues/141

PR-URL: https://github.com/nodejs/node/pull/28021
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
2019-06-17 08:27:17 -04:00

15 lines
412 B
JavaScript

'use strict';
const common = require('../common');
const { Worker } = require('worker_threads');
const w = new Worker(`
const dns = require('dns');
dns.lookup('nonexistent.org', () => {});
require('worker_threads').parentPort.postMessage('0');
`, { eval: true });
w.on('message', common.mustCall(() => {
// This should not crash the worker during a DNS request.
w.terminate().then(common.mustCall());
}));