Age | Commit message (Collapse) | Author |
|
Adds a new abstract class for module authors and embedders to register
arbitrary isolates with `node::MultiIsolatePlatform`.
PR-URL: https://github.com/nodejs/node/pull/30324
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Joyee Cheung <joyeec9h3@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>
|
|
While V8 itself should not have any remaining tasks on the queue
during platform shutdown, our inspector implementation may do so.
Thus, the checks verifying that no tasks are queued at that point
make some of the inspector tasks flaky.
Remove the checks and replace them by explicitly destroying all
tasks that are left.
Refs: https://github.com/nodejs/node/pull/25653
Refs: https://github.com/nodejs/node/pull/28870#issuecomment-531908090
PR-URL: https://github.com/nodejs/node/pull/29587
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
|
|
This removes the implementations of NodePlatform::CallOnForegroundThread
and NodePlatform::CallDelayedOnForegroundThread and updates the
test_platform cctest to stop using them.
PR-URL: https://github.com/nodejs/node/pull/27872
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
|
|
Its intended that *-inl.h header files are only included into the src
files that call the inline methods. Explicitly include it into the files
that need it.
PR-URL: https://github.com/nodejs/node/pull/27631
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
|
|
We never execute tasks in a nested fashion, so enabling them should
be as simple as forwarding tasks to the existing `Post*` methods.
PR-URL: https://github.com/nodejs/node/pull/27252
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
|
|
PR-URL: https://github.com/nodejs/node/pull/26885
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
|
|
PR-URL: https://github.com/nodejs/node/pull/26909
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
|
|
- Since this was introduced, V8 has effectively started requiring
that the platform knows of the `Isolate*` before we (or an embedder)
create our `IsolateData` structure; therefore, (un)registering it
from the `IsolateData` constructor/destructor doesn’t make much
sense anymore.
- Instead, we can require that the register/unregister functions
are only called once, simplifying the implementation a bit.
- Add a callback that we can use to know when the platform has
cleaned up its resources associated with a given `Isolate`.
In particular, this means that in the Worker code, we don’t need
to rely on what are essentially guesses about the number of event
loop turns that we need in order to have everything cleaned up.
PR-URL: https://github.com/nodejs/node/pull/26384
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
|
|
PR-URL: https://github.com/nodejs/node/pull/26493
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
|
|
Instead of repeating the same `#ifdef DEBUG` + `SealHandleScope`
pattern over and over, create an utility that does this for us.
PR-URL: https://github.com/nodejs/node/pull/26459
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
|
|
Platform tasks should have their own handle scopes, rather than
leak into outer ones.
PR-URL: https://github.com/nodejs/node/pull/26376
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
|
|
There is no real reason to assume that V8 tasks would have
to run in a Node.js `Context`.
PR-URL: https://github.com/nodejs/node/pull/26376
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
|
|
PR-URL: https://github.com/nodejs/node/pull/26294
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
|
|
Insert a NULLCHECK prior to return. Ideally we do this in the caller,
but the TraceController object is somewhat special as:
1. It is accessed by most threads
2. It's life cycle is managed by Agent::Agent
3. It's getter is invoked through Base Methods (upstream)
Refs: https://github.com/nodejs/node/issues/25814
PR-URL: https://github.com/nodejs/node/pull/25943
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Masashi Hirano <shisama07@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
|
|
PR-URL: https://github.com/nodejs/node/pull/25916
Refs: https://github.com/nodejs/node/pull/20914
Reviewed-By: Masashi Hirano <shisama07@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
|
|
Node first calls `Isolate::Dispose`, then
`NodePlatform::UnregisterIsolate`.
This again calls `PerIsolatePlatformData::Shutdown`, which (before this
patch) called `FlushForegroundTasksInternal`, which might call
`RunForegroundTask` if it finds foreground tasks to be executed. This
will fail however, since `Isolate::GetCurrent` was already reset during
`Isolate::Dispose`.
Hence remove the check to `FlushForegroundTasksInternal` and add checks
instead that no more foreground tasks are scheduled.
Refs: https://github.com/v8/node/pull/86
PR-URL: https://github.com/nodejs/node/pull/25653
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
|
|
Minor cleanup in the lifetime for the platform worker initialization
synchronization barrier.
PR-URL: https://github.com/nodejs/node/pull/23419
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
|
|
Metadata trace-events should be held in secondary storage so that they
can be periodically reemitted. This change establishes the secondary
storage and ensures that events are reemitted on each flush.
PR-URL: https://github.com/nodejs/node/pull/20900
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
|
|
For safer shutdown, we should destroy the platform – and background
threads - before the tracing infrastructure is destroyed. This change
fixes the relative order of NodePlatform disposition and the tracing
agent shutting down. This matches the nesting order for startup.
Make the tracing agent own the tracing controller instead of platform
to match the above.
Fixes: https://github.com/nodejs/node/issues/22865
PR-URL: https://github.com/nodejs/node/pull/22938
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
|
|
Make sure background workers are ready before proceeding with the
bootstrap or post-bootstrap execution of any code that may trigger
`process.exit()`.
Fixes: https://github.com/nodejs/node/issues/23065
PR-URL: https://github.com/nodejs/node/pull/23233
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
|
|
PR-URL: https://github.com/nodejs/node/pull/21983
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
|
|
This method is crucial for Runtime.evaluate protocol command with
timeout flag. At least Chrome DevTools frontend uses this method for
every execution in console.
PR-URL: https://github.com/nodejs/node/pull/22383
Fixes: https://github.com/nodejs/node/issues/22157
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
|
|
Refs: https://github.com/nodejs/node/pull/21079#discussion_r194251183
PR-URL: https://github.com/nodejs/node/pull/21982
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
|
|
Precursor to removing deprecated APIs on the v8 side @
https://chromium-review.googlesource.com/c/v8/v8/+/1045310
PR-URL: https://github.com/nodejs/node/pull/21079
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yang Guo <yangguo@chromium.org>
|
|
This commit removes the normal header file include if an internal one
is specified as per the CPP_STYLE_GUIDE.
PR-URL: https://github.com/nodejs/node/pull/21381
Reviewed-By: Matheus Marchini <matheus@sthima.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Ujjwal Sharma <usharma1998@gmail.com>
|
|
We use the `Isolate*` pointer as the sole identifier
for a V8 Isolate. In some environments (e.g. multi-threaded),
Isolates may be destroyed and new ones created; then, it
may happen that the memory that was previously used for
one `Isolate` can be re-used for another `Isolate`
after the first one has been disposed of.
This check is a little guard against accidentally
re-using the same per-Isolate platform data structure
in such cases, i.e. making sure (to the degree to which
that is possible) that the old `Isolate*` has been properly
unregistered before one at the same memory address is added.
(It’s not 100 % foolproof because the `uv_loop_t*`
pointer value could theoretically be the same as well.)
PR-URL: https://github.com/nodejs/node/pull/21156
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
|
|
This commit adds a rule to cpplint to check that pointers in the code
base lean to the left and not right, and also fixes the violations
reported.
PR-URL: https://github.com/nodejs/node/pull/21010
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
|
|
Currently the following compiler warnings are generated:
In file included from ../src/node_platform.cc:1:
../src/node_platform.h:83:16:
warning: private field 'isolate_' is not used [-Wunused-private-field]
v8::Isolate* isolate_;
^
1 warning generated.
This commit removes these unused private member.
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>
|
|
Clean up once all references to an `Isolate*` are gone from the
`NodePlatform`, rather than waiting for the `PerIsolatePlatformData`
struct to be deleted since there may be cyclic references between
that struct and the individual tasks.
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>
|
|
V8 uses a thread pool provided by the host to schedule background tasks
for concurrent GC and compiation. Emit trace events to identify the
background threads. Ensure that the tracing infrastructure is started
before the thread pool is initialized.
PR-URL: https://github.com/nodejs/node/pull/20823
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
|
|
Foreground tasks that repost themselves can force the draining loop
to run indefinitely long without giving other tasks chance to run.
This limits the foreground task draining loop to run only the tasks
that were in the tasks queue at the beginning of the loop.
PR-URL: https://github.com/nodejs/node/pull/19987
Fixes: https://github.com/nodejs/node/issues/19937
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yang Guo <yangguo@chromium.org>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Khaidi Chu <i@2333.moe>
|
|
It is required by a change in V8.
Refs: https://chromium-review.googlesource.com/c/v8/v8/+/598666
PR-URL: https://github.com/nodejs/node/pull/16271
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
|
|
V8 is switching APIs for scheduling tasks. Implement the new APIs.
Fixes: https://github.com/nodejs/node-v8/issues/24
Refs: https://github.com/v8/v8/commit/c690f54d9580243c53f7d892fcff1ce6bae4bfc0
PR-URL: https://github.com/nodejs/node/pull/17134
Fixes: https://github.com/nodejs/node-v8/issues/24
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
|
|
Use std::unique_ptr for delayed tasks in the scheduled
delayed tasks vector. This makes it clear that the vector
has ownership of the delayed tasks and is responsible for
deleting them.
Use a custom deleter for the pointers because libuv
needs to close the handle and then delete the data. Provide
the handle when creating the pointer instead of invoking the
special delete action everytime an element is removed from the vector.
PR-URL: https://github.com/nodejs/node/pull/17083
Reviewed-By: Anna Henningsen <anna@addaleax.net>
|
|
Replace raw pointers in task queues with std::unique_ptr. This
makes ownership obvious.
PR-URL: https://github.com/nodejs/node/pull/16970
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
|
|
Worker threads need an event loop without active libuv handles in
order to shut down. One source of handles that was previously
not accounted for were delayed V8 tasks; these create timers
that would be standing in the way of clearing the event loop.
To solve this, keep track of the scheduled tasks in a list
and close their timer handles before the corresponding isolate/loop
is removed from the platform.
It is not clear from the V8 documentation what the expectation is
with respect to pending background tasks at the end of the
isolate lifetime; however, an alternative approach of executing
these scheduled tasks when flushing them led to an infinite loop
of tasks scheduling each other; so it seems safe to assume that
the behaviour implemented in this patch is at least acceptable.
Original-PR-URL: https://github.com/ayojs/ayo/pull/120
Original-Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
PR-URL: https://github.com/nodejs/node/pull/16700
Reviewed-By: James M Snell <jasnell@gmail.com>
|
|
This splits the task queue used for asynchronous tasks scheduled
by V8 in per-isolate queues, so that multiple threads can be supported.
Original-PR-URL: https://github.com/ayojs/ayo/pull/89
Original-Reviewed-By: Timothy Gu <timothygu99@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/16700
Reviewed-By: James M Snell <jasnell@gmail.com>
|
|
https://github.com/nodejs/node/pull/15428 was supposed to account
for upcoming changes in V8 upstream, but while addressing review
comments a bug was introduced; `DrainBackgroundTasks()` should
always at least perform one blocking drain on the background task
queue.
PR-URL: https://github.com/nodejs/node/pull/15639
Refs: https://github.com/nodejs/node/pull/15428
Refs: f27b5e4bdaafc73a830a0451ee3c641b8bcd08fe
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
|
|
V8 platform tasks may schedule other tasks (both background and
foreground), and may perform asynchronous operations like
resolving Promises.
To address that:
- Run the task queue drain call inside a callback scope.
This makes sure asynchronous operations inside it, like
resolving promises, lead to the microtask queue and any
subsequent operations not being silently forgotten.
- Move the task queue drain call before `EmitBeforeExit()`
and only run `EmitBeforeExit()` if there is no new event
loop work.
- Account for possible new foreground tasks scheduled by
background tasks in `DrainBackgroundTasks()`.
PR-URL: https://github.com/nodejs/node/pull/15428
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Matthew Loring <mattloring@google.com>
|
|
Node.js currently uses the V8 implementation of the DefaultPlatform
which schedules VM tasks on a V8 managed thread pool. Since the Node.js
event loop is not aware of these tasks, the Node.js process may exit
while there are outstanding VM tasks. This will become problematic once
asynchronous wasm compilation lands in V8.
This PR introduces a Node.js specific implementation of the v8::Platform
on top of libuv so that the event loop is aware of outstanding VM tasks.
PR-URL: https://github.com/nodejs/node/pull/14001
Fixes: https://github.com/nodejs/node/issues/3665
Fixes: https://github.com/nodejs/node/issues/8496
Fixes: https://github.com/nodejs/node/issues/12980
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
|