[v12.x backport] src: improve embedder API #35241

Make the calls `stop_sub_worker_contexts()`, `RunCleanup()` part of the public API for easier embedding. (Note that calling `RunAtExit()` is idempotent because the at-exit callback queue is cleared after each call.) PR-URL: nodejs#30467 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]>

PR-URL: nodejs#30467 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]>

Workers are fully in control of their Isolates, and this helps avoid a problem with later changes to `CreateEnvironment()` because now we can run the boostrapping code inside the latter. PR-URL: nodejs#30467 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]>

In our codebase, the assumption generally is that `!is_main_thread()` means that the current Environment belongs to a Node.js Worker thread. PR-URL: nodejs#30467 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]>

This addresses some long-standing TODOs by Joyee and me about making the embedder API more powerful and us less reliant on internal APIs for creating the main thread and Workers. PR-URL: nodejs#30467 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]>

This allows embedders to flexibly control how they start JS code rather than using `third_party_main`. PR-URL: nodejs#30467 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]>

Allow passing a string as the main module rather than using the callback variant. PR-URL: nodejs#30467 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]>

Refs: nodejs#31910 PR-URL: nodejs#30467 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]>

This makes this bit of the embedder situation a bit easier to use. PR-URL: nodejs#30467 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]>

This is a decent replacement for the to-be-deprecated Init() API. PR-URL: nodejs#30467 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]>

This should eventually remove any necessity to use the global-state `GetMainThreadMultiIsolatePlatform()`. PR-URL: nodejs#30467 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]>

We do not need a Node.js-provided `v8::TracingController`, generally. Loosen that restriction in order to make it easier for embedders to provide their own subclass of `v8::TracingController`, or none at all. Refs: electron/electron@9c36576 PR-URL: nodejs#30467 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]>

If created platform with CreatePlatform, the crash occurs because it does not check if it was initialized to v8_platform when DisposePlatform was called. Refs: nodejs#31260 Co-authored-by: Anna Henningsen <[email protected]> PR-URL: nodejs#30467 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]>

There is currently no way to properly do this. PR-URL: nodejs#30467 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]>

PR-URL: nodejs#30467 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]>

Add an embedder cctest that also covers a multi-Environment situation, including worker_threads-style inspector support. Co-authored-by: Joyee Cheung <[email protected]> PR-URL: nodejs#30467 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]>

I’ve seen this fail a few times in CI, presumably because the inspector commmand did not reach the child thread in time. Explicitly waiting seems to solve that. Refs: nodejs#30467 PR-URL: nodejs#32563 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>

Embedders may not want to terminate the process when `process.exit()` is called. This provides a hook for more flexible handling of that situation. Refs: nodejs#30467 (comment) PR-URL: nodejs#32531 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: James M Snell <[email protected]>

This makes sense given that terminating execution of the parent thread this way likely also is supposed to stop all running Worker threads spawned by it. PR-URL: nodejs#32531 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: James M Snell <[email protected]>

Refs: nodejs@d7f1107 Fixes: nodejs#30257 PR-URL: nodejs#32406 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Jiawen Geng <[email protected]> Reviewed-By: Matheus Marchini <[email protected]> Reviewed-By: David Carlier <[email protected]> Reviewed-By: James M Snell <[email protected]>

This un-breaks testing in the case of `./configure --debug-node`. PR-URL: nodejs#32422 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: James M Snell <[email protected]>

This is necessary for `--inspect-brk-node` to work, and for the inspector to be aware of scripts created before bootstrapping. Fixes: nodejs#32648 Refs: nodejs#30467 (comment) PR-URL: nodejs#32672 Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Eugene Ostroukhov <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Reviewed-By: David Carlier <[email protected]> Reviewed-By: James M Snell <[email protected]>

Mostly, this introduces a pattern that makes sure that if a custom error is reported, `stopped_` will be set to `true` correctly in every cast, which was previously missing for the `NewContext().IsEmpty()` case (which led to a hard crash from the `Worker` destructor). This also leaves TODO comments for a few cases in which `ERR_WORKER_OUT_OF_MEMORY` was not used in accordance with the documentation for that error code (or according to its intention). Fixing that is semver-major. PR-URL: nodejs#33084 Reviewed-By: Gireesh Punathil <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>

Otherwise this potentially leads to race conditions when used in a multi-threaded environment (i.e. when used for its very purpose). PR-URL: nodejs#32523 Reviewed-By: Matheus Marchini <[email protected]> Reviewed-By: James M Snell <[email protected]>

Fix this to account for the fact that `Stop()` may already have been called from a cleanup hook when the `inspector::Agent` is deleted along with the `Environment`, at which point cleanup hooks are no longer available. PR-URL: nodejs#32523 Reviewed-By: Matheus Marchini <[email protected]> Reviewed-By: James M Snell <[email protected]>

This cleans up `Agent::RequestIoThreadStart()` significantly. PR-URL: nodejs#32523 Reviewed-By: Matheus Marchini <[email protected]> Reviewed-By: James M Snell <[email protected]>

This simplifies the code significantly, and removes the dependency of the inspector code on the availability of a `MultiIsolatePlatform` (by removing the dependency on a platform altogether). It also fixes a memory leak that occurs when `RequestInterrupt()` is used, but the interrupt handler is never called before the Isolate is destroyed. One downside is that this leads to a slight change in timing, because inspector messages are not dispatched in a re-entrant way. This means having to harden one test to account for that possibility by making sure that the stack is always clear through a `setImmediate()`. This does not affect the assertion made by the test, which is that messages will not be delivered synchronously while other code is executing. nodejs#32415 PR-URL: nodejs#32523 Reviewed-By: Matheus Marchini <[email protected]> Reviewed-By: James M Snell <[email protected]>

This avoids an edge-case memory leak. Refs: nodejs#32523 (comment) PR-URL: nodejs#32523 Reviewed-By: Matheus Marchini <[email protected]> Reviewed-By: James M Snell <[email protected]>

This ensures that microtasks scheduled by native immediates are run after the tasks are done. In particular, this affects the inspector integration since 6f9f546. Fixes: nodejs#33002 Refs: nodejs#32523 PR-URL: nodejs#34366 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Gerhard Stöbich <[email protected]> Reviewed-By: James M Snell <[email protected]>

libuv 1.39.0 will begin requiring uv_setup_args() to be called before attempting to access the process title. This commit adds uv_setup_args() calls that were missing in order for the test suite to pass (and updates the documentation). PR-URL: nodejs#34751 Reviewed-By: Richard Lau <[email protected]> Reviewed-By: David Carlier <[email protected]> Reviewed-By: Rich Trott <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[v12.x backport] src: improve embedder API #35241

[v12.x backport] src: improve embedder API #35241

Commits on Sep 23, 2020