diff options
| author | Anna Henningsen <anna@addaleax.net> | 2019-06-02 15:09:57 +0200 |
|---|---|---|
| committer | Anna Henningsen <anna@addaleax.net> | 2019-06-17 08:27:17 -0400 |
| commit | ed24c19002379063ce100037bfff7ca2da5b6cc8 (patch) | |
| tree | ffecdf23eb3adcfe72a626c129fcc715d477fc20 /src/node_worker.h | |
| parent | 10a346edde47c872de9afbcb79aa43095064078b (diff) | |
| download | node-new-ed24c19002379063ce100037bfff7ca2da5b6cc8.tar.gz | |
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>
Diffstat (limited to 'src/node_worker.h')
| -rw-r--r-- | src/node_worker.h | 1 |
1 files changed, 0 insertions, 1 deletions
diff --git a/src/node_worker.h b/src/node_worker.h index 0e659a9f52..db3c95d2ae 100644 --- a/src/node_worker.h +++ b/src/node_worker.h @@ -52,7 +52,6 @@ class Worker : public AsyncWrap { static void Unref(const v8::FunctionCallbackInfo<v8::Value>& args); private: - void OnThreadStopped(); void CreateEnvMessagePort(Environment* env); std::shared_ptr<PerIsolateOptions> per_isolate_opts_; |
