This patch
1. Refactors the bootstrap routine of the main instance so that
when --no-node-snapshot is used,
Environment::InitializeMainContext() will only be called once
(previously it would be called twice, which was harmless for now
but not ideal).
2. Mark the number of BaseObjects in RunBootstrapping() when creating
the Environment from scratch and in InitializeMainContext() when
the Environment is deserialized. Previously the marking was done in
the Environment constructor and InitializeMainContext() respectively
for the cctest which was incorrect because the cctest never uses
an Environment that's not bootstrapped. Also renames the mark
to base_object_created_after_bootstrap to reflect what it's
intended for.
PR-URL: https://github.com/nodejs/node/pull/37113
Refs: https://github.com/nodejs/node/pull/36943
Reviewed-By: James M Snell <jasnell@gmail.com>
Provide helpers for a) spinning the event loop and
b) setting up and tearing down the objects involved in a single
Node.js instance, as they would typically be used.
The former helper is also usable inside Node.js itself,
for both Worker and main threads.
PR-URL: https://github.com/nodejs/node/pull/35597
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
When a process exits cleanly, i.e. because the event loop ends up
without things to wait for, the Node.js objects that are left on
the heap should be:
1. weak, i.e. ready for garbage collection once no longer
referenced, or
2. detached, i.e. scheduled for destruction once no longer
referenced, or
3. an unrefed libuv handle, i.e. does not keep the event loop
alive, or
4. an inactive libuv handle (essentially the same here)
There are a few exceptions to this rule, but generally,
if there are C++-backed Node.js objects on the heap
that do not fall into the above categories, we may be looking
at a potential memory leak. Most likely, the cause is a missing
`MakeWeak()` call on the corresponding object.
In order to avoid this kind of problem, we check the list
of BaseObjects for these criteria. In this commit, we only do so
when explicitly instructed to or when in debug mode
(where --verify-base-objects is always-on).
In particular, this avoids the kinds of memory leak issues
that were fixed in the PRs referenced below.
Refs: https://github.com/nodejs/node/pull/35488
Refs: https://github.com/nodejs/node/pull/35487
Refs: https://github.com/nodejs/node/pull/35481
PR-URL: https://github.com/nodejs/node/pull/35490
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Add an ExternalReferenceRegistry class for registering static
external references.
To register the external JS to C++ references created in a binding
(e.g. when a FunctionTemplate is created):
- Add the binding name (same as the id used for `internalBinding()`
and `NODE_MODULE_CONTEXT_AWARE_INTERNAL`) to
`EXTERNAL_REFERENCE_BINDING_LIST` in `src/node_external_reference.h`.
- In the file where the binding is implemented, create a registration
function to register the static C++ references (e.g. the C++
functions in `v8::FunctionCallback` associated with the function
templates), like this:
```c++
void RegisterExternalReferences(
ExternalReferenceRegistry* registry) {
registry->Register(cpp_func_1);
}
```
- At the end of the file where `NODE_MODULE_CONTEXT_AWARE_INTERNAL` is
also usually called, register the registration function with
```
NODE_MODULE_EXTERNAL_REFERENCE(binding_name,
RegisterExternalReferences);
```
PR-URL: https://github.com/nodejs/node/pull/32984
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
This commit removes the unused 'misc' variable from one of the
NodeMainInstance constructors.
Another option could be to add a default argument to
SetIsolateMiscHandlers but I'd like to hear what others think about that
first.
PR-URL: https://github.com/nodejs/node/pull/33417
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
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: https://github.com/nodejs/node/pull/30467
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
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: https://github.com/nodejs/node/pull/30467
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
This PR exposes a new embedder-focused API: SetIsolateUpForNode.
It maintains previous behavior for the single-param version of
SetIsolateUpForNode and changes no defaults, but was designed to be
flexible by allowing for embedders to conditionally override all
callbacks and flags set by the previous two-param version of
SetIsolateUpForNode.
PR-URL: https://github.com/nodejs/node/pull/30150
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Fold `CancelPendingDelayedTasks()` into `UnregisterIsolate()` and
make implementing it optional.
It makes sense for these two operations to happen at the same time,
so it is sufficient to provide a single operation instead of two
separate ones.
PR-URL: https://github.com/nodejs/node/pull/30034
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Shelley Vohr <codebytere@gmail.com>
- Split the initialization of the inspector and other diagnostics
into `Environment::InitializeInspector()` and
`Environment::InitializeDiagnostics()` - these need to be
reinitialized separately after snapshot deserialization.
- Do not store worker url alongside the inspector parent handle,
instead just get it from the handle.
- Rename `Worker::profiler_idle_notifier_started_` to
`Worker::start_profiler_idle_notifier_` because it stores
the state inherited from the parent env to use for initializing
itself.
PR-URL: https://github.com/nodejs/node/pull/27539
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Inline `ProcessCliArgs()` in the `Environment` constructor, and
emit the `Environment` creation trace events with the arguments
earlier. Remove the unused arguments passed to `CreateProcessObject()`
since these are now attached to process in `PatchProcessObject()`
during pre-execution instead.
PR-URL: https://github.com/nodejs/node/pull/27539
Reviewed-By: Anna Henningsen <anna@addaleax.net>
This patch encapsulates the main isolate management into a
NodeMainInstance class that manages the resources with RAII
and controls the Isolate::CreateParams (which is necessary
for deserializing snapshots with external references)
PR-URL: https://github.com/nodejs/node/pull/27220
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Joyee Cheung