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 aPromise
. - 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
@nodejs/workers @benjamingr
Checklist
-
make -j4 test
(UNIX), orvcbuild test
(Windows) passes -
tests and/or benchmarks are included -
documentation is changed or added -
commit message follows commit guidelines