This PR makes a change to the Worker.terminate() when called if the
kHandler is null. Before this pull request it was returning undefined,
but the API is expecting a promise. With the changes in this PR if
terminate is called a Promise.resolve() is returned, unless a callback
is passed in which case the old behavior stays (returns undefined).
PR-URL: https://github.com/nodejs/node/pull/28370
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
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>
This caches the current working directory and only updates the variable
if `process.chdir()` is called.
PR-URL: https://github.com/nodejs/node/pull/27224
Reviewed-By: John-David Dalton <john.david.dalton@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
`debuglog()` depends on `process.pid` and `process.env.NODE_DEBUG`,
so it needs to be called lazily in top scopes of internal modules
that may be loaded before these run time states are allowed to
be accessed. This patch makes its implementation lazy by default,
the process states are only accessed when the returned debug
function is called for the first time.
PR-URL: https://github.com/nodejs/node/pull/27281
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Replace a couple of checks with a single regular expression.
PR-URL: https://github.com/nodejs/node/pull/27233
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Instead of sharing the OS-backed store for all `process.env` instances,
create a copy of `process.env` for every worker that is created.
The copies do not interact. Native-addons do not see modifications to
`process.env` from Worker threads, but child processes started from
Workers do default to the Worker’s copy of `process.env`.
This makes Workers behave like child processes as far as `process.env`
is concerned, and an option corresponding to the `child_process`
module’s `env` option is added to the constructor.
Fixes: https://github.com/nodejs/node/issues/24947
PR-URL: https://github.com/nodejs/node/pull/26544
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
This patch splits the implementation of util.debuglog into a
separate file and explicitly initialize it during pre-execution
since the initialization depends on environment variables.
Also delays the call to `debuglog` in modules that are loaded during
bootstrap to make sure we only access the environment variable
during pre-execution.
PR-URL: https://github.com/nodejs/node/pull/26468
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: https://github.com/nodejs/node/pull/25871
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yuta Hiroto <hello@hiroppy.me>
Reviewed-By: James M Snell <jasnell@gmail.com>
Embedders may want to control whether a Node.js instance
controls the current process, similar to what we currently
have with `Worker`s.
Previously, the `isMainThread` flag had a bit of a double usage,
both for indicating whether we are (not) running a Worker and
whether we can modify per-process state.
PR-URL: https://github.com/nodejs/node/pull/25881
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
- Assign thread IDs to `Environment` instances, rather than Workers.
This is more embedder-friendly than the current system, in which
all “main threads” (if there are multiple ones) would get the
id `0`.
- Because that means that `isMainThread === (threadId === 0)` no longer
holds, refactor `isMainThread` into a separate entity. Implement it
in a way that allows for future extensibility, because we use
`isMainThread` in multiple different ways (determining whether there
is a parent thread; determining whether the current thread has control
of the current process; etc.).
PR-URL: https://github.com/nodejs/node/pull/25796
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
This prevents a main thread from rewriting the policy file and loading
a worker that has a different policy from the main thread.
PR-URL: https://github.com/nodejs/node/pull/25710
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
This PR adds the ability to provide Workers with their own
execArgv flags in replacement of the main thread's execArgv. Only
per-Isolate/per-Environment options are allowed. Per-Process options
and V8 flags are not allowed. Passing an empty execArgv array will
reset per-Isolate and per-Environment options of the Worker to their
defaults. If execArgv option is not passed, the Worker will get
the same flags as the main thread.
Usage example:
```
const worker = new Worker(__filename, {
execArgv: ['--trace-warnings'],
});
```
PR-URL: https://github.com/nodejs/node/pull/25467
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Move worker bootstrap code into worker_thread_only.js from
internal/worker.js since they are only run once during bootstrap.
PR-URL: https://github.com/nodejs/node/pull/25199
Reviewed-By: James M Snell <jasnell@gmail.com>
- Move `setupProcessStdio` which contains write access to
the process object into `bootstrap/node.js`
- Move `MessagePort`, `MessageChannel`, `ReadableWorkerStdio`,
and `WritableWorkerStdio` into `internal/worker/io.js`
- Move more worker-specific bootstrap code into
`internal/process/worker_thread_only` from `setupChild`
in `internal/worker.js`, and move the `process._fatalException`
overwrite into `bootstrap/node.js` for clarity.
PR-URL: https://github.com/nodejs/node/pull/25199
Reviewed-By: James M Snell <jasnell@gmail.com>
This patch:
- Moves `tryGetCwd`, `evalScript` and `fatalException` from
`bootstrap/node.js` into `process/execution.js` so that
they do have to be passed into the worker thread
setup function, instead the worker code can require them
when necessary.
- Moves `setUncaughtExceptionCaptureCallback` and
`hasUncaughtExceptionCaptureCallback` along with the two
global state `exceptionHandlerState` and
`shouldAbortOnUncaughtToggle` info `process.execution.js`
as those are only used by the fatalException and these
two accessors as one self-contained unit.
PR-URL: https://github.com/nodejs/node/pull/25199
Reviewed-By: James M Snell <jasnell@gmail.com>
When the worker thread exits, drain the messages also from the internal
message port so that the call to 'kDispose' will occur only after all
the messages from the worker were processed in the parent, so stdio
messages from the worker will be successfully pushed to their target
streams in the parent.
PR-URL: https://github.com/nodejs/node/pull/24932
Fixes: https://github.com/nodejs/node/issues/24636
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
internalBinding is used so often that it should just automatically be
available for usage in internals.
PR-URL: https://github.com/nodejs/node/pull/23025
Refs: https://github.com/nodejs/node/commit/2a9eb31
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
For all classes descending from `AsyncWrap`, use JS inheritance
instead of manually adding methods to the individual classes.
This allows cleanup of some code around transferring handles
over IPC.
PR-URL: https://github.com/nodejs/node/pull/23094
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
`MessagePort` is special because it has to be a C++ API
that is exposed to userland. Therefore, there is a number
of internal methods on its native prototype; this commit
reduces this set of methods to only what is documented in
the API.
PR-URL: https://github.com/nodejs/node/pull/23037
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
There is no need to call `setupPortReferencing` in `setupChild`
as which has been called with the same arguments in the `oninit`
prototype method of `MessagePort`.
PR-URL: https://github.com/nodejs/node/pull/22298
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Previously even after uncaught exception the worker would continue to
execute until there is no more work to do.
PR-URL: https://github.com/nodejs/node/pull/21739
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
* set process.exitCode before calling 'exit' handlers so that there will
not be a situation where process.exitCode !== code in 'exit' callback
during uncaughtException handling
* don't ignore process.exitCode set in 'exit' callback when failed with
uncaughtException and there is no uncaughtException listener
PR-URL: https://github.com/nodejs/node/pull/21739
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Now worker.on('exit') reports correct exit code (1) if worker has exited
with uncaught exception.
Fixes: https://github.com/nodejs/node/issues/21707
PR-URL: https://github.com/nodejs/node/pull/21713
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Yuta Hiroto <hello@hiroppy.me>
Reviewed-By: Weijia Wang <starkwang@126.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
This change can prevent typos and redundant strings in code.
PR-URL: https://github.com/nodejs/node/pull/21537
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
This commit adds support for relative paths in Worker.
Paths are relative to the current working directory.
PR-URL: https://github.com/nodejs/node/pull/21407
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Rather than passing errors using some sort of string representation,
do a best effort for faithful serialization/deserialization of
uncaught exception objects.
PR-URL: https://github.com/nodejs/node/pull/20876
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Shingo Inoue <leko.noor@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: John-David Dalton <john.david.dalton@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Provide `stdin`, `stdout` and `stderr` options for the `Worker`
constructor, and make these available to the worker thread
under their usual names.
The default for `stdin` is an empty stream, the default for
`stdout` and `stderr` is redirecting to the parent thread’s
corresponding stdio streams.
PR-URL: https://github.com/nodejs/node/pull/20876
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Shingo Inoue <leko.noor@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: John-David Dalton <john.david.dalton@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Implement multi-threading support for most of the API.
Thanks to Stephen Belanger for reviewing this change in its
original form, to Olivia Hugger for reviewing the
documentation and some of the tests coming along with it,
and to Alexey Orlenko and Timothy Gu for reviewing other
parts of the tests.
Refs: https://github.com/ayojs/ayo/pull/110
Refs: https://github.com/ayojs/ayo/pull/114
Refs: https://github.com/ayojs/ayo/pull/117
PR-URL: https://github.com/nodejs/node/pull/20876
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Shingo Inoue <leko.noor@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: John-David Dalton <john.david.dalton@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Implement `MessagePort` and `MessageChannel` along the lines of
the DOM classes of the same names. `MessagePort`s initially
support transferring only `ArrayBuffer`s.
Thanks to Stephen Belanger for reviewing this change in its
original form, to Benjamin Gruenbaum for reviewing the
added tests in their original form, and to Olivia Hugger
for reviewing the documentation in its original form.
Refs: https://github.com/ayojs/ayo/pull/98
PR-URL: https://github.com/nodejs/node/pull/20876
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Shingo Inoue <leko.noor@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: John-David Dalton <john.david.dalton@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
