Age | Commit message (Collapse) | Author |
|
Since async hooks are per-environment and putting them in
the environment allows us to serialize them for the
snapshot automatically.
PR-URL: https://github.com/nodejs/node/pull/39112
Refs: https://github.com/nodejs/node/pull/38905
Refs: https://github.com/nodejs/node/issues/35711
Reviewed-By: Anna Henningsen <anna@addaleax.net>
|
|
PR-URL: https://github.com/nodejs/node/pull/37986
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
|
|
This fixes more C4244 MSVC warnings in the code base.
Refs: https://github.com/nodejs/node/pull/37149
PR-URL: https://github.com/nodejs/node/pull/37334
Reviewed-By: James M Snell <jasnell@gmail.com>
|
|
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.
https://github.com/nodejs/node/issues/32415
PR-URL: https://github.com/nodejs/node/pull/32523
Reviewed-By: Matheus Marchini <mat@mmarchini.me>
Reviewed-By: James M Snell <jasnell@gmail.com>
|
|
I found it exceedingly hard to figure out if there is a race condition
where one thread reads the inspector agent's HostPort's properties while
another modifies them concurrently.
I think the answer is "no, there isn't" but with this commit use sites
are forced to unwrap the object (and acquire the mutex in the process),
making it a great deal easier to reason about correctness.
PR-URL: https://github.com/nodejs/node/pull/31717
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: David Carlier <devnexen@gmail.com>
|
|
This API is designed to enable worker threads use Inspector protocol
on main thread (and other workers through NodeWorker domain).
Note that worker can cause dead lock by suspending itself. I will
work on a new API that will allow workers to be hidden from the
inspector.
Fixes: https://github.com/nodejs/node/issues/28828
PR-URL: https://github.com/nodejs/node/pull/28870
Reviewed-By: Aleksei Koziatinskii <ak239spb@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
|
|
This is a cleanup, allowing for a better separation of concerns.
PR-URL: https://github.com/nodejs/node/pull/28526
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Aleksei Koziatinskii <ak239spb@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
|
|
Previously the enhancement were done right after emitting
`'uncaughtException'`, which meant by the time we knew the
exception was fatal in C++, the error.stack had already been
patched.
This patch moves those routines to be called later during the
fatal exception handling, and split them into two stages:
before and after the inspector is notified by the invocation of
`V8Inspector::exceptionThrown`. We now expand the stack to include
additional informations about unhandled 'error' events before
the inspector is notified, but delay the highlighting of the
frames until after the inspector is notified, so that the
ANSI escape sequences won't show up in the inspector console.
PR-URL: https://github.com/nodejs/node/pull/28308
Fixes: https://github.com/nodejs/node/issues/28287
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
|
|
Fix unnecessary inclusion of node_options-inl.h into env.h via
inspector_agent.h, causing almost all of src/ to recompile on any change
to the options parser. Its intended that *-inl.h header files are only
included into the src files that call the inline methods.
PR-URL: https://github.com/nodejs/node/pull/27538
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
|
|
`v8::Global` is essentially a nicer variant of `node::Persistent` that,
in addition to reset-on-destroy, also implements move semantics.
This commit makes the necessary replacements, removes
`node::Persistent` and (now-)unnecessary inclusions of the
`node_persistent.h` header, and makes some of the functions that
take Persistents as arguments more generic so that they work with all
`v8::PersistentBase` flavours.
PR-URL: https://github.com/nodejs/node/pull/27287
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
|
|
PR-URL: https://github.com/nodejs/node/pull/27264
Reviewed-By: Refael Ackermann <refack@gmail.com>
|
|
PR-URL: https://github.com/nodejs/node/pull/26306
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
|
|
PR-URL: https://github.com/nodejs/node/pull/26159
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
|
|
Refs: https://github.com/nodejs/node/issues/24016
PR-URL: https://github.com/nodejs/node/pull/26011
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
|
|
Instead of using a shared pointer of the entire debug option set,
pass the parsed debug option to inspector classes by value because
they are set once the CLI argument parsing is done. Add another shared
pointer to HostPort being used by the inspector server, which is copied
from the one in the debug options initially. The port of the shared
HostPort is 9229 by default and can be specified as 0 initially but
will be set to the actual port of the server once it starts listening.
This makes the shared state clearer and makes it possible to use
`require('internal/options')` in JS land to query the CLI options
instead of using `process._breakFirstLine` and other underscored
properties of `process` since we are now certain that these
values should not be altered once the parsing is done and can be
passed around in copies without locks.
PR-URL: https://github.com/nodejs/node/pull/24772
Reviewed-By: Anna Henningsen <anna@addaleax.net>
|
|
PR-URL: https://github.com/nodejs/node/pull/23302
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
|
|
Introduce a NodeTarget inspector domain modelled after ChromeDevTools
Target domain. It notifies inspector frontend attached to a main V8
isolate when workers are starting and allows passing messages to
inspectors on their isolates. All inspector functionality is enabled on
worker isolates.
PR-URL: https://github.com/nodejs/node/pull/21364
Reviewed-By: Aleksei Koziatinskii <ak239spb@gmail.com>
Reviewed-By: Jan Krems <jan.krems@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
|
|
PR-URL: https://github.com/nodejs/node/pull/22769
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
|
|
We generally add these to all headers that are considered
internal to Node.
These aren’t distributed as part of the headers tarball,
so I think this does not have to be semver-major
(and we have been changing the APIs in these headers
freely anyway).
PR-URL: https://github.com/nodejs/node/pull/22514
Reviewed-By: James M Snell <jasnell@gmail.com>
|
|
This is a major refactor of our Node’s parser. See `node_options.cc`
for how it is used, and `node_options-inl.h` for the bulk
of its implementation.
Unfortunately, the implementation has come to have some
complexity, in order to meet the following goals:
- Make it easy to *use* for defining or changing options.
- Keep it (mostly) backwards-compatible.
- No tests were harmed as part of this commit.
- Be as consistent as possible.
- In particular, options can now generally accept arguments
through both `--foo=bar` notation and `--foo bar` notation.
We were previously very inconsistent on this point.
- Separate into different levels of scope, namely
per-process (global), per-Isolate and per-Environment
(+ debug options).
- Allow programmatic accessibility in the future.
- This includes a possible expansion for `--help` output.
This commit also leaves a number of `TODO` comments, mostly for
improving consistency even more (possibly with having to modify
tests), improving embedder support, as well as removing pieces of
exposed configuration variables that should never have become
part of the public API but unfortunately are at this point.
PR-URL: https://github.com/nodejs/node/pull/22392
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
|
|
Workers debugging will require interfacing between the "main" inspector
and per-worker isolate inspectors. This is consistent with what WS
interface does. This change is a refactoring change and does not change
the functionality.
PR-URL: https://github.com/nodejs/node/pull/21182
Fixes: https://github.com/nodejs/node/issues/21725
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
|
|
It is now easily accessible from the Environment
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
|
|
This change adds a new inspector domain for receiving Node tracing
data.
1. Node.js now can extend Inspector protocol with new domains with
the API defined in the src/inspector/node_protocol.pdl.
2. Plumbing code will be generated at the build time. /json/protocol
HTTP endpoint returns both V8 and Node.js inspector protocol.
3. "NodeTracing" domain was introduced. It is based on the Chrome
"Tracing" domain.
PR-URL: https://github.com/nodejs/node/pull/20608
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
|
|
This change enables concurrent inspector sessions, through WebSocket
interface as well as JS interface, in any combination.
PR-URL: https://github.com/nodejs/node/pull/20137
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
|
|
Replace v8::Persistent with node::Persistent, a specialization that
resets the persistent handle on destruction. Prevents accidental
resource leaks when forgetting to call .Reset() manually.
I'm fairly confident this commit fixes a number of resource leaks that
have gone undiagnosed so far.
PR-URL: https://github.com/nodejs/node/pull/18656
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
|
|
PR-URL: https://github.com/nodejs/node/pull/17939
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: James M Snell <jasnell@gmail.com>
|
|
The `auxData` field is not exposed to JavaScript, as DevTools uses it
for its `isDefault` parameter, which is implemented faithfully,
contributing to the nice indentation in the context selection panel.
Without the indentation, when `Target` domain gets implemented (along
with a single Inspector for cluster) in #16627, subprocesses and VM
contexts will be mixed up, causing confusion.
PR-URL: https://github.com/nodejs/node/pull/17720
Refs: https://github.com/nodejs/node/pull/14231#issuecomment-315924067
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: James M Snell <jasnell@gmail.com>
|
|
Attaching WS session will now include a roundtrip onto the main thread
to make sure there is no other session (e.g. JS bindings)
This change also required refactoring WS socket implementation to better
support scenarios like this.
Fixes: https://github.com/nodejs/node/issues/16852
PR-URL: https://github.com/nodejs/node/pull/17085
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
|
|
Fix a regression that was introduced in commit 5886e204f0 ("inspector:
track async stacks when necessary") and that I overlooked during review:
the persistent handle with the callback must be rematerialized *after*
the `v8::HandleScope` is created, not before.
Apparently `test/sequential/test-inspector-async-call-stack.js` has
no test coverage for this scenario and I'm out of good ideas on how
to create a concise and reliable test case.
Fixes: https://github.com/nodejs/node/issues/17496
PR-URL: https://github.com/nodejs/node/pull/17539
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Khaidi Chu <i@2333.moe>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
|
|
Include the header instead of using a forward declaration.
PR-URL: https://github.com/nodejs/node/pull/16677
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
|
|
With this change, we do async stack tracking only when explicitly
requested by the inspector client. This avoids unnecessary overhead
for clients that might not be interested in async stack traces.
PR-URL: https://github.com/nodejs/node/pull/16308
Fixes: https://github.com/nodejs/node/issues/16180
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
|
|
PR-URL: https://github.com/nodejs/node/pull/16056
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.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>
|
|
Implement a special async_hooks listener that forwards information
about async tasks to V8Inspector asyncTask* API, thus enabling
DevTools feature "async stack traces".
The feature is enabled only on 64bit platforms due to a technical
limitation of V8 Inspector: inspector uses a pointer as a task id,
while async_hooks use 64bit numbers as ids.
To avoid performance penalty of async_hooks when not debugging,
the new listener is enabled only when the process enters a debug mode:
- When the process is started with `--inspect` or `--inspect-brk`,
the listener is enabled immediately and async stack traces
lead all the way to the first tick of the event loop.
- When the debug mode is enabled via SIGUSR1 or `_debugProcess()`,
the listener is enabled together with the debugger. As a result,
only async operations started after the signal was received
will be correctly observed and reported to V8 Inspector. For example,
a `setInterval()` called in the first tick of the event will not be
shown in the async stack trace when the callback is invoked. This
behaviour is consistent with Chrome DevTools.
Last but not least, this commit fixes handling of InspectorAgent's
internal property `enabled_` to ensure it's set back to `false`
after the debugger is deactivated (typically via `process._debugEnd()`).
Fixes: https://github.com/nodejs/node/issues/11370
PR-URL: https://github.com/nodejs/node/pull/13870
Reviewed-by: Timothy Gu <timothygu99@gmail.com>
Reviewed-by: Anna Henningsen <anna@addaleax.net>
|
|
This enables inspector support for contexts created using the vm module.
PR-URL: https://github.com/nodejs/node/pull/14465
Reviewed-By: Jan Krems <jan.krems@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org>
|
|
PR-URL: https://github.com/nodejs/node/pull/13228
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
|
|
This change allows reentering the message dispatch loop when the Node is
paused. This is necessary when the pause happened as a result of the
message sent by a debug frontend, such as evaluating a function with a
breakpoint inside.
Fixes: https://github.com/nodejs/node/issues/13320
PR-URL: https://github.com/nodejs/node/pull/13350
Reviewed-By: James M Snell <jasnell@gmail.com>
|
|
Pure refactor, makes no functional changes but the renaming helped me
see more clearly what the relationship was between methods and
variables.
* Renamed methods to reduce number of slightly different names for the
same thing ("thread" vs "io thread", etc.).
* Added comments where it was useful to me.
PR-URL: https://github.com/nodejs/node/pull/13321
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
|
|
This fixes a resource leak detected by a Coverity scan.
PR-URL: https://github.com/nodejs/node/pull/13198
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Aleksei Koziatinskii <ak239spb@gmail.com>
|
|
PR-URL: https://github.com/nodejs/node/pull/12263
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: Josh Gavant <josh.gavant@outlook.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
|
|
This change ensures that the WebSocket server can be stopped
(and restarted if needed) buy calling process._debugEnd.
PR-URL: https://github.com/nodejs/node/pull/12777
Fixes: https://github.com/nodejs/node/issues/12559
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
|
|
'inspector' property is not an official API and should not be published
on process object, where the user may discover it.
This change was extracted from https://github.com/nodejs/node/pull/12263
that will be focused on creating JS bindings.
PR-URL: https://github.com/nodejs/node/pull/12656
Reviewed-By: Aleksey Kozyatinskiy <kozyatinskiy@chromium.org>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
|
|
AgentImpl was introduce so inspector-agent.h does not leak libuv and
inspector implementation details. Inspector had been reworked since and
new classes provide this isolation and AgentImpl became unnecessary
level of indirection. This change removes that class to streamline the
code.
PR-URL: https://github.com/nodejs/node/pull/12576
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
|
|
This pull request switches the signal handler to start inspector socket
server instead of the legacy V8 debug protocol.
PR-URL: https://github.com/nodejs/node/pull/11431
Fixes: https://github.com/nodejs/node/issues/8464
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
|
|
This change removes a need for using deprecated debug context for
breaking at the start of the main module.
PR-URL: https://github.com/nodejs/node/pull/12076
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
|
|
As inspector functionality expands, more options will need to be added.
Currently this requires changing adding function arguments, etc. This
change packs the veriables into a single class that can be extended
without changing APIs.
PR-URL: https://github.com/nodejs/node/pull/9691
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
|
|
Both our team experiments and some embedder request indicate a potential
in implementing alternative transport for inspector - e.g. IPC pipes or
custom embedder APIs. This change moves all HTTP specific code into a
separate class and is a first attempt at defining a boundary between the
inspector agent and transport. This API will be refined as new
transports are implemented.
Note that even without considering alternative transports, this change
enables better testing of the HTTP server (Valgrind made it possible to
identify and fix some existing memory leaks).
PR-URL: https://github.com/nodejs/node/pull/9630
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
|
|
Changes inspector integration to use Node.js script file name as target
title (reported in JSON and shown in developer tools UIs). It will also
report file:// URL for the script as some tools seem to use that field
to open the script in the editor.
PR-URL: https://github.com/nodejs/node/pull/8243
Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl>
|
|
To output exception in DevTools console method exceptionThrown should
be called on uncaught exception on V8Inspector object. Additionally
we need to wait disconnect to provide user way to inspect exception.
PR-URL: https://github.com/nodejs/node/pull/8043
Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: jasnell - James M Snell <jasnell@gmail.com>
Reviewed-By: ofrobots - Ali Ijaz Sheikh <ofrobots@google.com>
|
|
Node process will no longer terminate with an assertion if the
inspector port is not available.
PR-URL: https://github.com/nodejs/node/pull/7874
Reviewed-By: bnoordhuis - Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: jasnell - James M Snell <jasnell@gmail.com>
|