Once `worker.terminate()` is called, the Worker instance will be
destroyed as soon as possible anyway, so in order to make
the Promise returned by `worker.terminate()` resolve always,
it should be okay to just call `.ref()` on it and keep the main
event loop alive temporarily.
PR-URL: https://github.com/nodejs/node/pull/29484
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
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>
`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>
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>
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 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 `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>
`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>
* 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>
Michaël Zasso