Commit Graph
22 Commits
Author SHA1 Message Date
Joyee Cheung d2a13695bf src: track cppgc wrappers with a list in Realm
This allows us to perform cleanups of cppgc wrappers that rely on a
living Realm during Realm shutdown. Otherwise the cleanup may happen
during object destruction, which can be triggered by GC after Realm
shutdown, leading to invalid access to Realm.

The general pattern for this type of non-trivial destruction is
designed to be:

```
class MyWrap final : CPPGC_MIXIN(MyWrap) {
    public:
    ~MyWrap() { this->Finalize(); }
    void Clean(Realm* realm) override {
        // Do cleanup that relies on a living Realm. This would be
        // called by CppgcMixin::Finalize() first during Realm
        // shutdown, while the Realm is still alive. If the destructor
        // calls Finalize() again later during garbage collection that
        // happens after Realm shutdown, Clean() would be skipped,
        // preventing invalid access to the Realm.
    }
}
```

In addition, this allows us to trace external memory held by the
wrappers in the heap snapshots if we add synthethic edges between the
wrappers and other nodes in the embdder graph callback, or to perform
snapshot serialization for them.

PR-URL: https://github.com/nodejs/node/pull/56534
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
2025-05-19 20:56:33 +00:00
Chengzhong Wu 316ac8c250 src: add memory retainer traits for external types
Add `MemoryRetainerTraits` to reveal external type memory info without
forcing them to inherit from `MemoryRetainer`.

PR-URL: https://github.com/nodejs/node/pull/56881
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
2025-02-04 17:07:11 +00:00
Joyee Cheung 3646a66044 src: fix AliasedBuffer memory attribution in heap snapshots
Before the AliasedBuffers were represented solely by the TypedArrays
event though they also retain additional memory themselves.
Make the accounting more accurate by implementing MemoryRetainer in
AliasedBuffer.

PR-URL: https://github.com/nodejs/node/pull/46817
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
2023-03-01 17:45:04 +01:00
Joyee Cheung 8b2126f63f src: move AliasedBuffer implementation to -inl.h
Drive-by: Replace the SFINAE with a static_assert because we don't
have (or need) an implementation for non-scalar AliasedBufferBase
otherwise. Add forward declarations to memory_tracker.h now that
aliased-buffer.h no longer includes util-inl.h.

PR-URL: https://github.com/nodejs/node/pull/46817
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
2023-03-01 17:45:02 +01:00
Chengzhong Wu 80fa25bf60 src: use constant strings for memory info names
PR-URL: https://github.com/nodejs/node/pull/46087
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Darshan Sen <raisinten@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
2023-01-09 16:39:48 +00:00
Joyee Cheung 3ab2d4f362 src: implement GetDetachedness() in MemoryRetainerNode
This allows us to mark weak/detached references in the heap snapshot.
Also mark weak/detached BaseObject with Detachedness::kDetached
so that the state of the reference can be displayed by frontend
consuming the heap snapshot.

PR-URL: https://github.com/nodejs/node/pull/44803
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
2022-10-05 19:38:39 +08:00
Anna Henningsen bafd5444ab src: do not track BaseObjects via cleanup hooks
Since f59ec2abee, `BaseObject` instances were tracked in heap snapshots
through their associated `CleanupHookCallback`s which were stored on
the `Environment`; however, this is inaccurate, because:

- Edges in heap dumps imply a keeps-alive relationship, but cleanup
  hooks do not keep the `BaseObject`s that they point to alive.
- It loses information about whether `BaseObject` instances are
  GC roots: Even weak `BaseObject`s are now, practically speaking,
  showing up as hanging off a GC root when that isn’t actually the case
  (e.g. in the description of nodejs/node#33468).

Thus, this is a partial revert of f59ec2abee.

Refs: https://github.com/nodejs/node/issues/33468
Refs: https://github.com/nodejs/node/pull/27018

PR-URL: https://github.com/nodejs/node/pull/33809
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
2020-06-11 23:39:49 +02:00
Anna Henningsen e68d4c6f5f src: allow unique_ptrs with custom deleter in memory tracker
Originally landed in the QUIC repo

Original review metadata:

```
  PR-URL: https://github.com/nodejs/quic/pull/145
  Reviewed-By: James M Snell <jasnell@gmail.com>
```

PR-URL: https://github.com/nodejs/node/pull/31870
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
2020-02-24 07:57:25 -08:00
legendecas 7b7e7bd185 src,lib: make ^C print a JS stack trace
If terminating the process with ctrl-c / SIGINT, prints a JS stacktrace
leading up to the currently executing code.

The feature would be enabled under option `--trace-sigint`.

Conditions of no stacktrace on sigint:

- has (an) active sigint listener(s);
- main thread is idle (i.e. uv polling), a message instead of stacktrace
  would be printed.

PR-URL: https://github.com/nodejs/node/pull/29207
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Christopher Hiller <boneskull@boneskull.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
2020-01-28 13:52:27 +08:00
Anna Henningsen 7e6104f1bb src: introduce custom smart pointers for BaseObjects
Referring to `BaseObject` instances using standard C++ smart pointers
can interfere with BaseObject’s own cleanup mechanisms
(explicit delete, delete-on-GC and delete-on-cleanup).

Introducing custom smart pointers allows referring to `BaseObject`s
safely while keeping those mechanisms intact.

Refs: https://github.com/nodejs/quic/pull/141
Refs: https://github.com/nodejs/quic/pull/149
Reviewed-By: James M Snell <jasnell@gmail.com>

PR-URL: https://github.com/nodejs/node/pull/30374
Refs: https://github.com/nodejs/quic/pull/165
Reviewed-By: David Carlier <devnexen@gmail.com>
2019-11-19 13:47:29 +01:00
Anna Henningsen 2bdeb88c27 src: remove custom tracking for SharedArrayBuffers
Remove custom tracking for `SharedArrayBuffer`s and their allocators
and instead let V8 do the tracking of both. This is required starting
in V8 7.9, because lifetime management for `ArrayBuffer::Allocator`s
differs from what was performed previously (i.e. it is no longer
easily possible for one Isolate to release an `ArrayBuffer` and another
to accept it into its own allocator), and the alternative would
have been adapting the `SharedArrayBuffer` tracking logic to also
apply to regular `ArrayBuffer` instances.

Refs: https://github.com/nodejs/node/pull/30044

PR-URL: https://github.com/nodejs/node/pull/30020
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
2019-11-08 15:53:36 +01:00
Anna Henningsen 723d5c058f src: prefer v8::Global over node::Persistent
`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>
2019-04-30 00:23:33 +02:00
Joyee Cheung 0ae46a7a86 src: use predefined AliasedBuffer types in the code base
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>
2019-04-26 14:23:31 +08:00
gengjiawen 51079db826 src: apply clang-tidy rule modernize-use-equals-default
PR-URL: https://github.com/nodejs/node/pull/27264
Reviewed-By: Refael Ackermann <refack@gmail.com>
2019-04-24 13:32:43 -04:00
Joyee Cheung f59ec2abee src: implement MemoryRetainer in Environment
This allows us to track the essentially-global objects in
Environment in the heap snapshot. Note that this patch only
tracks the fields that can be tracked correctly. There are
still several types of fields that cannot be tracked:

- v8::Data including v8::Private, v8::ObjectTemplate etc.
- Internal types that do not implement MemoryRetainer yet
- STL containers with MemoryRetainer* inside
- STL containers with numeric types inside that should not have their
  nodes elided e.g. numeric keys in maps.

The `BaseObject`s are now no longer globals. They are tracked
as arguments in CleanupHookCallbacks referenced by the Environment
node. This model is closer to how their lifetime is managed
internally.

To track the per-environment strong persistent properties, this patch
divides them into those that are also `v8::Value` and those that
are just `v8::Data`. The values can be tracked by the current
memory tracker while the data cannot.

This patch also implements the `MemoryRetainer` interface in several
internal classes so that they can be tracked in the heap snapshot.

PR-URL: https://github.com/nodejs/node/pull/27018
Refs: https://github.com/nodejs/node/issues/26776
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
2019-04-04 11:09:29 +08:00
Refael Ackermann f2064dfc1f src: de-lint header usage
PR-URL: https://github.com/nodejs/node/pull/26306
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
2019-03-12 09:57:19 -04:00
Joyee Cheung f3d6207874 doc: fix the example implementation of MemoryRetainer
We need to be careful not to include the size of non-pointer
fields in the parent's self size if we want to track them separately
as a different node.

Refs: https://github.com/nodejs/node/pull/26161/files#r259170771

PR-URL: https://github.com/nodejs/node/pull/26262
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
2019-03-06 17:48:55 +01:00
Gireesh Punathil 17b4949a84 src: track memory retainer fields
If retainers are embedded in retainers, direct tracking
those lead to double tracking. Instead, use a special tracker
that adjusts the tracking for the container object.

PR-URL: https://github.com/nodejs/node/pull/26161
Reviewed-By: Anna Henningsen <anna@addaleax.net>
2019-02-28 10:02:33 +05:30
Gabriel Schulhof 0603c0a53f src: bundle persistent-to-local methods as class
Create a class `PersistentToLocal` which contains three methods,
`Strong`, `Weak`, and `Default`:

* `Strong` returns a `Local` from a strong persistent reference,
* `Weak` returns a `Local` from a weak persistent reference, and
* `Default` decides based on `IsWeak()` which of the above two to call.

These replace `node::StrongPersistentToLocal()`,
`node::WeakPersistentToLocal()`, and `node::PersistentToLocal()`,
respectively.

PR-URL: https://github.com/nodejs/node/pull/24276
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
2018-11-13 05:27:40 +01:00
Joyee Cheung 92fa0fcdb7 src: name EmbededderGraph edges and use class names for nodes
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>
2018-10-04 15:32:30 +02:00
Anna Henningsen 266a7e6258 src: use V8 graph heap snapshot API
Transition to a newer, more flexible API for
heap snapshot creation.

This addresses a currently pending deprecation in the V8 API.

PR-URL: https://github.com/nodejs/node/pull/21741
Fixes: https://github.com/nodejs/node/issues/21633
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
2018-07-15 20:35:35 +02:00
Anna Henningsen 57e301539b src: enable more detailed memory tracking
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>
2018-07-13 19:53:15 +02:00