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>
Instead of allowing the callers to instantiate the template
with any numeric types (such as aliasing a Uint8Array to double[]),
predefine types that make sense and use those instead.
PR-URL: https://github.com/nodejs/node/pull/27334
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Also creates `CreateMainEnvironment` to encapsulate the code
creating the main environment from the provided Isolate data
and arguments.
PR-URL: https://github.com/nodejs/node/pull/26788
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Refael Ackermann <refack@gmail.com>
(Re-?)add a public API for creating linked bindings (access to
`NM_F_LINKED` as a constant was previously removed in
d6ac8a4db0), and add a test for
the functionality.
PR-URL: https://github.com/nodejs/node/pull/26457
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Shelley Vohr <codebytere@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Make sure `node::Init()` is called once, and execute
`RunBootstrapping()` so that Node’s internals are ready
when the cctests run.
PR-URL: https://github.com/nodejs/node/pull/26457
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Shelley Vohr <codebytere@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/26284
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Previously only simple escape sequences were handled
(i.e. \n, \t, r etc.). This commit adds escaping of other control
symbols in the range of 0x00 to 0x20.
Also, this replaces multiple find+replace calls with a single pass
replacer.
PR-URL: https://github.com/nodejs/node/pull/25626
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Richard Lau <riclau@uk.ibm.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>
This patch:
- Refactors the `MemoryRetainer` API so that the impementer no longer
calls `TrackThis()` that sets the size of node on the top of the
stack, which may be hard to understand. Instead now they implements
`SelfSize()` to provide their self sizes. Also documents
the API in the header.
- Refactors `MemoryTracker` so it calls `MemoryInfoName()` and
`SelfSize()` of `MemoryRetainer` to retrieve info about them, and
separate `node_names` and `edge_names` so the edges can be properly
named with reference names and the nodes can be named with class
names. (Previously the nodes are named with reference names while the
edges are all indexed and appear as array elements).
- Adds `SET_MEMORY_INFO_NAME()`, `SET_SELF_SIZE()` and
`SET_NO_MEMORY_INFO()` convenience macros
- Fixes a few `MemoryInfo` calls in some `MemoryRetainers` to track
their references properly.
- Refactors the heapdump tests to check both node names and edge names,
distinguishing between wrapped JS nodes (without prefixes)
and embedder wrappers (prefixed with `Node / `).
PR-URL: https://github.com/nodejs/node/pull/23072
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Make `Environment::GetCurrent()` return `nullptr` if the current
`Context` is not a Node.js context, and for the relevant usage of
this function, either:
- Switch to the better `GetCurrent(args)` variant
- Turn functions in to no-ops where it makes sense
- Make it a `CHECK`, i.e. an API requirement, where it make sense
- Leave a `TODO` comment for verifying what, if anything, is to be done
PR-URL: https://github.com/nodejs/node/pull/22819
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
This commit adds an InitializeV8Platform function which calls
v8_platform's Initialize to create the NodePlatform and also set the
structs members.
When running cctests this functions was not being called (it is called
from the Start function but that function is not called by the test
fixture.
The motivation for adding this is that I'm guessing that embedders
would might need the ability to do the same thing.
Refs: https://github.com/nodejs/node-v8/issues/69
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>
Skip one specific test that requires ICU
Fixes: https://github.com/nodejs/node/issues/19051
PR-URL: https://github.com/nodejs/node/pull/22265
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: https://github.com/nodejs/node/pull/21971
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
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>
This will enable more detailed heap snapshots based on
a newer V8 API.
This commit itself is not tied to that API and could
be backported.
PR-URL: https://github.com/nodejs/node/pull/21742
Reviewed-By: James M Snell <jasnell@gmail.com>
Port of the V8 internal v8::tracing::TracedValue that allows
structured data to be included in the trace event. The v8 class
is not exported in the public API so we cannot use it directly.
This is a simplified and slightly modified port. This commit only
adds the class, it does not add uses of it. Those will come in
separate PRs/commits.
PR-URL: https://github.com/nodejs/node/pull/21475
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
This commit updates the gtest fixture to use node::NewIsolate instead of
creating a new V8 Isolate using v8::Isolate::New.
The motivation for this is that without calling node::NewIsolate the
various callbacks set on the isolate, for example AddMessageListener,
SetFatalErrorHandler etc, would not get set. I don't think this is the
expected behaviour and I ran into this when writing a new cctest.
PR-URL: https://github.com/nodejs/node/pull/21419
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Refactor Timers to behave more similarly to Immediates by having
a single uv_timer_t handle which is stored on the Environment.
No longer expose timers in a public binding and instead make
it part of the internalBinding.
PR-URL: https://github.com/nodejs/node/pull/20894
Fixes: https://github.com/nodejs/node/issues/10154
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Remove some unused code from the WS server implementation and switch to
smart pointers where possible.
PR-URL: https://github.com/nodejs/node/pull/21070
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
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>
No need to heap-allocate values here.
PR-URL: https://github.com/nodejs/node/pull/20665
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
- Instead of storing a pointer whose type refers to the specific
subclass of `BaseObject`, just store a `BaseObject*` directly.
This means in particular that one can cast to classes along
the way of the inheritance chain without issues, and that
`BaseObject*` no longer needs to be the first superclass
in the case of multiple inheritance.
In particular, this renders hack-y solutions to this problem (like
ddc19be6de) obsolete and addresses
a `TODO` comment of mine.
- Move wrapping/unwrapping methods to the `BaseObject` class.
We use these almost exclusively for `BaseObject`s, and I hope
that this gives a better idea of how (and for what) these are used
in our code.
- Perform initialization/deinitialization of the internal field
in the `BaseObject*` constructor/destructor. This makes the code
a bit more obviously correct, avoids explicit calls for this
in subclass constructors, and in particular allows us to avoid
crash situations when we previously called `ClearWrap()`
during GC.
This also means that we enforce that the object passed to the
`BaseObject` constructor needs to have an internal field.
This is the only reason for the test change.
- Change the signature of `MakeWeak()` to not require a pointer
argument. Previously, this would always have been the same
as `this`, and no other value made sense. Also, the parameter
was something that I personally found somewhat confusing
when becoming familiar with Node’s code.
- Add a `TODO` comment that motivates switching to real inheritance
for the JS types we expose from the native side. This patch
brings us a lot closer to being able to do that.
- Some less significant drive-by cleanup.
Since we *effectively* already store the `BaseObject*` pointer
anyway since ddc19be6de, I do not
think that this is going to have any impact on diagnostic tooling.
Fixes: https://github.com/nodejs/node/issues/18897
PR-URL: https://github.com/nodejs/node/pull/20455
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Daniel Bevenius <daniel.bevenius@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>
Node instance may not know the real host and port user sees when
debug frontend connects through the SSH tunnel. This change fixes
'/json/list' response by using the value client provided in the host
header.
PR-URL: https://github.com/nodejs/node/pull/19664
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
CleanupHandles() has not been called in our own code base anymore
after the v8 debug agent has been removed. It used to be in the
~Environment() destructor but then removed to avoid firing other
events after the exit event, given that we were not going to
clean up handles for the one environment per process setup.
Call it in FreeEnvironment so that embedders can clean up
the handles in the loop when creating multiple environments.
PR-URL: https://github.com/nodejs/node/pull/19319
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Jackson Tian <shyvo1987@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This commit adds operator overloads for increment/decrement to
AliasedBuffer::Reference.
The motivation for doing this is to hopefully make code that needs
to increment/decrement a little simpler.
PR-URL: https://github.com/nodejs/node/pull/19083
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Currently the cctest target depend on the node_core_target_name
target. But it is the node_lib_target_name target that compiles the
sources now which means that if a source file in src is updated the
cctest executable will not be re-linked against it, but will remain
unchanged. The code will still be compiled, just not linked which
means that if you are debugging you'll not see the changes and also a
warning will be displayed about this issue.
This commit changes the cctest target to depend on node_lib_target_name.
PR-URL: https://github.com/nodejs/node/pull/18576
Reviewed-By: Matheus Marchini <matheus@sthima.com>
Reviewed-By: Yihong Wang <yh.wang@ibm.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
This commit set the Environment as a thread local when CreateEnvironment
is called which is currently not being done. This would lead to a
segment fault if later node::AtExit is called without specifying the
environment parameter. This specific issue was reported by Electron.
If I recall correctly, back when this was implemented the motivation was
that if embedders have multiple environments per isolate they should be
using the AtExit functions that take an environment. This is not the
case with Electron which only create a single environment (as far as I
know), and if a native module calls AtExit this would lead to the
segment fault.
I was able to reproduce Electron issue and the provided test simulates
it. I was also able to use this patch and verify that it works for the
Electron issue as well.
PR-URL: https://github.com/nodejs/node/pull/18573
Refs: https://github.com/nodejs/node/pull/9163
Refs: https://github.com/electron/electron/issues/11299
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Matheus Marchini <matheus@sthima.com>
For tests that use anonymous namespaces, some tagged the close
of the namespace with 'namespace' while others used
'anonymous namespace'. It was suggested I should use
'anonymous namespace' in a recent PR review so make all of the
tests consistent with this.
PR-URL: https://github.com/nodejs/node/pull/18583
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
This commit add SetUpTestCase and TearDownTestCase functions that will
be called once per test case. Currently we only have SetUp/TearDown
which are called for each test.
This commit moves the initialization and configuration of Node and V8 to
be done on a per test case basis, but gives each test a new Isolate.
PR-URL: https://github.com/nodejs/node/pull/18558
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
