From 099159ce042cae3022d9050121752336df20b740 Mon Sep 17 00:00:00 2001 From: Daryl Haresign Date: Thu, 10 Aug 2023 20:43:51 -0400 Subject: [PATCH] src,tools: initialize cppgc This patch: - Initializes cppgc in InitializeOncePerProcess() when kNoInitializeCppgc is not set - Create a CppHeap and attach it to the Isolate when there isn't one already during IsolateData initialization. The CppHeap is detached and terminated when IsolateData is freed. - Publishes the cppgc headers in the tarball. This allows C++ addons to start using cppgc to manage objects. A helper node::SetCppgcReference() is also added to help addons enable cppgc tracing in a user-defined object. Co-authored-by: Joyee Cheung Refs: https://github.com/nodejs/node/issues/40786 PR-URL: https://github.com/nodejs/node/pull/45704 Backport-PR-URL: https://github.com/nodejs/node/pull/49187 Refs: https://docs.google.com/document/d/1ny2Qz_EsUnXGKJRGxoA-FXIE2xpLgaMAN6jD7eAkqFQ/edit Reviewed-By: Stephen Belanger Reviewed-By: Rafael Gonzaga PR-URL: https://github.com/nodejs/node/pull/48660 Refs: https://github.com/v8/v8/commit/93275031284c66be7852b13f1b18a8bbe7f3a0a9 --- src/env-inl.h | 26 ++++++++++ src/env.cc | 26 ++++++++++ src/env.h | 10 ++++ src/node.cc | 14 +++++ src/node.h | 25 ++++++++- src/node_main_instance.cc | 2 + src/node_worker.cc | 1 + test/addons/cppgc-object/binding.cc | 78 ++++++++++++++++++++++++++++ test/addons/cppgc-object/binding.gyp | 9 ++++ test/addons/cppgc-object/test.js | 51 ++++++++++++++++++ tools/install.py | 51 +++++++++++++++++- 11 files changed, 290 insertions(+), 3 deletions(-) create mode 100644 test/addons/cppgc-object/binding.cc create mode 100644 test/addons/cppgc-object/binding.gyp create mode 100644 test/addons/cppgc-object/test.js diff --git a/src/env-inl.h b/src/env-inl.h index 9d2d3bfd81308a..7802304b1891ae 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -34,6 +34,7 @@ #include "node_realm-inl.h" #include "util-inl.h" #include "uv.h" +#include "v8-cppgc.h" #include "v8.h" #include @@ -61,6 +62,31 @@ inline uv_loop_t* IsolateData::event_loop() const { return event_loop_; } +inline void IsolateData::SetCppgcReference(v8::Isolate* isolate, + v8::Local object, + void* wrappable) { + v8::CppHeap* heap = isolate->GetCppHeap(); + CHECK_NOT_NULL(heap); + v8::WrapperDescriptor descriptor = heap->wrapper_descriptor(); + uint16_t required_size = std::max(descriptor.wrappable_instance_index, + descriptor.wrappable_type_index); + CHECK_GT(object->InternalFieldCount(), required_size); + + uint16_t* id_ptr = nullptr; + { + Mutex::ScopedLock lock(isolate_data_mutex_); + auto it = + wrapper_data_map_.find(descriptor.embedder_id_for_garbage_collected); + CHECK_NE(it, wrapper_data_map_.end()); + id_ptr = &(it->second->cppgc_id); + } + + object->SetAlignedPointerInInternalField(descriptor.wrappable_type_index, + id_ptr); + object->SetAlignedPointerInInternalField(descriptor.wrappable_instance_index, + wrappable); +} + inline uint16_t* IsolateData::embedder_id_for_cppgc() const { return &(wrapper_data_->cppgc_id); } diff --git a/src/env.cc b/src/env.cc index 2d0f25c0080b8d..7e3d3aca2d5f96 100644 --- a/src/env.cc +++ b/src/env.cc @@ -37,6 +37,8 @@ using errors::TryCatchScope; using v8::Array; using v8::Boolean; using v8::Context; +using v8::CppHeap; +using v8::CppHeapCreateParams; using v8::EmbedderGraph; using v8::EscapableHandleScope; using v8::Function; @@ -61,6 +63,7 @@ using v8::TracingController; using v8::TryCatch; using v8::Undefined; using v8::Value; +using v8::WrapperDescriptor; using worker::Worker; int const ContextEmbedderTag::kNodeContextTag = 0x6e6f64; @@ -538,6 +541,14 @@ IsolateData::IsolateData(Isolate* isolate, // for embedder ID, V8 could accidentally enable cppgc on them. So // safe guard against this. DCHECK_NE(descriptor.wrappable_type_index, BaseObject::kSlot); + } else { + cpp_heap_ = CppHeap::Create( + platform, + CppHeapCreateParams{ + {}, + WrapperDescriptor( + BaseObject::kEmbedderType, BaseObject::kSlot, cppgc_id)}); + isolate->AttachCppHeap(cpp_heap_.get()); } // We do not care about overflow since we just want this to be different // from the cppgc id. @@ -565,6 +576,21 @@ IsolateData::IsolateData(Isolate* isolate, } } +IsolateData::~IsolateData() { + if (cpp_heap_ != nullptr) { + // The CppHeap must be detached before being terminated. + isolate_->DetachCppHeap(); + cpp_heap_->Terminate(); + } +} + +// Public API +void SetCppgcReference(Isolate* isolate, + Local object, + void* wrappable) { + IsolateData::SetCppgcReference(isolate, object, wrappable); +} + void IsolateData::MemoryInfo(MemoryTracker* tracker) const { #define V(PropertyName, StringValue) \ tracker->TrackField(#PropertyName, PropertyName()); diff --git a/src/env.h b/src/env.h index e3f67f1dfeaa66..c02fc6bd62dd78 100644 --- a/src/env.h +++ b/src/env.h @@ -62,6 +62,10 @@ #include #include +namespace v8 { +class CppHeap; +} + namespace node { namespace shadow_realm { @@ -136,6 +140,7 @@ class NODE_EXTERN_PRIVATE IsolateData : public MemoryRetainer { MultiIsolatePlatform* platform = nullptr, ArrayBufferAllocator* node_allocator = nullptr, const SnapshotData* snapshot_data = nullptr); + ~IsolateData(); SET_MEMORY_INFO_NAME(IsolateData) SET_SELF_SIZE(IsolateData) @@ -148,6 +153,10 @@ class NODE_EXTERN_PRIVATE IsolateData : public MemoryRetainer { uint16_t* embedder_id_for_cppgc() const; uint16_t* embedder_id_for_non_cppgc() const; + static inline void SetCppgcReference(v8::Isolate* isolate, + v8::Local object, + void* wrappable); + inline uv_loop_t* event_loop() const; inline MultiIsolatePlatform* platform() const; inline const SnapshotData* snapshot_data() const; @@ -229,6 +238,7 @@ class NODE_EXTERN_PRIVATE IsolateData : public MemoryRetainer { NodeArrayBufferAllocator* const node_allocator_; MultiIsolatePlatform* platform_; const SnapshotData* snapshot_data_; + std::unique_ptr cpp_heap_; std::shared_ptr options_; worker::Worker* worker_context_ = nullptr; bool is_building_snapshot_ = false; diff --git a/src/node.cc b/src/node.cc index 247bff863ab44f..e6be00eeb3c185 100644 --- a/src/node.cc +++ b/src/node.cc @@ -63,6 +63,8 @@ #endif // NODE_USE_V8_PLATFORM #include "v8-profiler.h" +#include "cppgc/platform.h" + #if HAVE_INSPECTOR #include "inspector/worker_inspector.h" // ParentInspectorHandle #endif @@ -1116,6 +1118,14 @@ InitializeOncePerProcessInternal(const std::vector& args, V8::Initialize(); } + if (!(flags & ProcessInitializationFlags::kNoInitializeCppgc)) { + v8::PageAllocator* allocator = nullptr; + if (result->platform_ != nullptr) { + allocator = result->platform_->GetPageAllocator(); + } + cppgc::InitializeProcess(allocator); + } + performance::performance_v8_start = PERFORMANCE_NOW(); per_process::v8_initialized = true; @@ -1135,6 +1145,10 @@ void TearDownOncePerProcess() { ResetSignalHandlers(); } + if (!(flags & ProcessInitializationFlags::kNoInitializeCppgc)) { + cppgc::ShutdownProcess(); + } + per_process::v8_initialized = false; if (!(flags & ProcessInitializationFlags::kNoInitializeV8)) { V8::Dispose(); diff --git a/src/node.h b/src/node.h index c560ace29bd2fe..ca01c42e8af484 100644 --- a/src/node.h +++ b/src/node.h @@ -261,6 +261,10 @@ enum Flags : uint32_t { kNoUseLargePages = 1 << 11, // Skip printing output for --help, --version, --v8-options. kNoPrintHelpOrVersionOutput = 1 << 12, + // Do not perform cppgc initialization. If set, the embedder must call + // cppgc::InitializeProcess() before creating a Node.js environment + // and call cppgc::ShutdownProcess() before process shutdown. + kNoInitializeCppgc = 1 << 13, // Emulate the behavior of InitializeNodeWithArgs() when passing // a flags argument to the InitializeOncePerProcess() replacement @@ -269,7 +273,7 @@ enum Flags : uint32_t { kNoStdioInitialization | kNoDefaultSignalHandling | kNoInitializeV8 | kNoInitializeNodeV8Platform | kNoInitOpenSSL | kNoParseGlobalDebugVariables | kNoAdjustResourceLimits | - kNoUseLargePages | kNoPrintHelpOrVersionOutput, + kNoUseLargePages | kNoPrintHelpOrVersionOutput | kNoInitializeCppgc, }; } // namespace ProcessInitializationFlags namespace ProcessFlags = ProcessInitializationFlags; // Legacy alias. @@ -1486,6 +1490,25 @@ void RegisterSignalHandler(int signal, bool reset_handler = false); #endif // _WIN32 +// Configure the layout of the JavaScript object with a cppgc::GarbageCollected +// instance so that when the JavaScript object is reachable, the garbage +// collected instance would have its Trace() method invoked per the cppgc +// contract. To make it work, the process must have called +// cppgc::InitializeProcess() before, which is usually the case for addons +// loaded by the stand-alone Node.js executable. Embedders of Node.js can use +// either need to call it themselves or make sure that +// ProcessInitializationFlags::kNoInitializeCppgc is *not* set for cppgc to +// work. +// If the CppHeap is owned by Node.js, which is usually the case for addon, +// the object must be created with at least two internal fields available, +// and the first two internal fields would be configured by Node.js. +// This may be superseded by a V8 API in the future, see +// https://bugs.chromium.org/p/v8/issues/detail?id=13960. Until then this +// serves as a helper for Node.js isolates. +NODE_EXTERN void SetCppgcReference(v8::Isolate* isolate, + v8::Local object, + void* wrappable); + } // namespace node #endif // SRC_NODE_H_ diff --git a/src/node_main_instance.cc b/src/node_main_instance.cc index 2ef56f80dfc8f6..e1e456cfad9325 100644 --- a/src/node_main_instance.cc +++ b/src/node_main_instance.cc @@ -68,6 +68,8 @@ NodeMainInstance::~NodeMainInstance() { return; } // This should only be done on a main instance that owns its isolate. + // IsolateData must be freed before UnregisterIsolate() is called. + isolate_data_.reset(); platform_->UnregisterIsolate(isolate_); isolate_->Dispose(); } diff --git a/src/node_worker.cc b/src/node_worker.cc index 478e80be505bca..900674bbe4c90e 100644 --- a/src/node_worker.cc +++ b/src/node_worker.cc @@ -11,6 +11,7 @@ #include "node_snapshot_builder.h" #include "permission/permission.h" #include "util-inl.h" +#include "v8-cppgc.h" #include #include diff --git a/test/addons/cppgc-object/binding.cc b/test/addons/cppgc-object/binding.cc new file mode 100644 index 00000000000000..1b70ff11dc561a --- /dev/null +++ b/test/addons/cppgc-object/binding.cc @@ -0,0 +1,78 @@ +#include +#include +#include +#include +#include +#include +#include + +class CppGCed : public cppgc::GarbageCollected { + public: + static uint16_t states[2]; + static constexpr int kDestructCount = 0; + static constexpr int kTraceCount = 1; + + static void New(const v8::FunctionCallbackInfo& args) { + v8::Isolate* isolate = args.GetIsolate(); + v8::Local js_object = args.This(); + CppGCed* gc_object = cppgc::MakeGarbageCollected( + isolate->GetCppHeap()->GetAllocationHandle()); + node::SetCppgcReference(isolate, js_object, gc_object); + args.GetReturnValue().Set(js_object); + } + + static v8::Local GetConstructor( + v8::Local context) { + auto ft = v8::FunctionTemplate::New(context->GetIsolate(), New); + auto ot = ft->InstanceTemplate(); + v8::WrapperDescriptor descriptor = + context->GetIsolate()->GetCppHeap()->wrapper_descriptor(); + uint16_t required_size = std::max(descriptor.wrappable_instance_index, + descriptor.wrappable_type_index); + ot->SetInternalFieldCount(required_size + 1); + return ft->GetFunction(context).ToLocalChecked(); + } + + CppGCed() = default; + + ~CppGCed() { states[kDestructCount]++; } + + void Trace(cppgc::Visitor* visitor) const { states[kTraceCount]++; } +}; + +uint16_t CppGCed::states[] = {0, 0}; + +void InitModule(v8::Local exports) { + v8::Isolate* isolate = v8::Isolate::GetCurrent(); + auto context = isolate->GetCurrentContext(); + + auto store = v8::ArrayBuffer::NewBackingStore( + CppGCed::states, + sizeof(uint16_t) * 2, + [](void*, size_t, void*) {}, + nullptr); + auto ab = v8::ArrayBuffer::New(isolate, std::move(store)); + + exports + ->Set(context, + v8::String::NewFromUtf8(isolate, "CppGCed").ToLocalChecked(), + CppGCed::GetConstructor(context)) + .FromJust(); + exports + ->Set(context, + v8::String::NewFromUtf8(isolate, "states").ToLocalChecked(), + v8::Uint16Array::New(ab, 0, 2)) + .FromJust(); + exports + ->Set(context, + v8::String::NewFromUtf8(isolate, "kDestructCount").ToLocalChecked(), + v8::Integer::New(isolate, CppGCed::kDestructCount)) + .FromJust(); + exports + ->Set(context, + v8::String::NewFromUtf8(isolate, "kTraceCount").ToLocalChecked(), + v8::Integer::New(isolate, CppGCed::kTraceCount)) + .FromJust(); +} + +NODE_MODULE(NODE_GYP_MODULE_NAME, InitModule) diff --git a/test/addons/cppgc-object/binding.gyp b/test/addons/cppgc-object/binding.gyp new file mode 100644 index 00000000000000..55fbe7050f18e4 --- /dev/null +++ b/test/addons/cppgc-object/binding.gyp @@ -0,0 +1,9 @@ +{ + 'targets': [ + { + 'target_name': 'binding', + 'sources': [ 'binding.cc' ], + 'includes': ['../common.gypi'], + } + ] +} diff --git a/test/addons/cppgc-object/test.js b/test/addons/cppgc-object/test.js new file mode 100644 index 00000000000000..6d45becba2700f --- /dev/null +++ b/test/addons/cppgc-object/test.js @@ -0,0 +1,51 @@ +'use strict'; + +// Flags: --expose-gc + +const common = require('../../common'); + +// Verify that addons can create GarbageCollected objects and +// have them traced properly. + +const assert = require('assert'); +const { + CppGCed, states, kDestructCount, kTraceCount, +} = require(`./build/${common.buildType}/binding`); + +assert.strictEqual(states[kDestructCount], 0); +assert.strictEqual(states[kTraceCount], 0); + +let array = []; +const count = 100; +for (let i = 0; i < count; ++i) { + array.push(new CppGCed()); +} + +globalThis.gc(); + +setTimeout(async function() { + // GC should have invoked Trace() on at least some of the CppGCed objects, + // but they should all be alive at this point. + assert.strictEqual(states[kDestructCount], 0); + assert.notStrictEqual(states[kTraceCount], 0); + + // Replace the old CppGCed objects with new ones, after GC we should have + // destructed all the old ones and called Trace() on the + // new ones. + for (let i = 0; i < count; ++i) { + array[i] = new CppGCed(); + } + await common.gcUntil( + 'All old CppGCed are destroyed', + () => states[kDestructCount] === count, + ); + // Release all the CppGCed objects, after GC we should have destructed + // all of them. + array = null; + globalThis.gc(); + + await common.gcUntil( + 'All old CppGCed are destroyed', + () => states[kDestructCount] === count * 2, + ); +}, 1); diff --git a/tools/install.py b/tools/install.py index f92cd74bcdcf6f..11616e1bcac530 100755 --- a/tools/install.py +++ b/tools/install.py @@ -197,15 +197,61 @@ def files(action): def headers(action): def wanted_v8_headers(files_arg, dest): v8_headers = [ + # The internal cppgc headers are depended on by the public + # ones, so they need to be included as well. + 'deps/v8/include/cppgc/internal/api-constants.h', + 'deps/v8/include/cppgc/internal/atomic-entry-flag.h', + 'deps/v8/include/cppgc/internal/base-page-handle.h', + 'deps/v8/include/cppgc/internal/caged-heap-local-data.h', + 'deps/v8/include/cppgc/internal/caged-heap.h', + 'deps/v8/include/cppgc/internal/compiler-specific.h', + 'deps/v8/include/cppgc/internal/finalizer-trait.h', + 'deps/v8/include/cppgc/internal/gc-info.h', + 'deps/v8/include/cppgc/internal/logging.h', + 'deps/v8/include/cppgc/internal/member-storage.h', + 'deps/v8/include/cppgc/internal/name-trait.h', + 'deps/v8/include/cppgc/internal/persistent-node.h', + 'deps/v8/include/cppgc/internal/pointer-policies.h', + 'deps/v8/include/cppgc/internal/write-barrier.h', + # cppgc headers + 'deps/v8/include/cppgc/allocation.h', 'deps/v8/include/cppgc/common.h', - 'deps/v8/include/libplatform/libplatform.h', + 'deps/v8/include/cppgc/cross-thread-persistent.h', + 'deps/v8/include/cppgc/custom-space.h', + 'deps/v8/include/cppgc/default-platform.h', + 'deps/v8/include/cppgc/ephemeron-pair.h', + 'deps/v8/include/cppgc/explicit-management.h', + 'deps/v8/include/cppgc/garbage-collected.h', + 'deps/v8/include/cppgc/heap-consistency.h', + 'deps/v8/include/cppgc/heap-handle.h', + 'deps/v8/include/cppgc/heap-state.h', + 'deps/v8/include/cppgc/heap-statistics.h', + 'deps/v8/include/cppgc/heap.h', + 'deps/v8/include/cppgc/liveness-broker.h', + 'deps/v8/include/cppgc/macros.h', + 'deps/v8/include/cppgc/member.h', + 'deps/v8/include/cppgc/name-provider.h', + 'deps/v8/include/cppgc/object-size-trait.h', + 'deps/v8/include/cppgc/persistent.h', + 'deps/v8/include/cppgc/platform.h', + 'deps/v8/include/cppgc/prefinalizer.h', + 'deps/v8/include/cppgc/process-heap-statistics.h', + 'deps/v8/include/cppgc/sentinel-pointer.h', + 'deps/v8/include/cppgc/source-location.h', + 'deps/v8/include/cppgc/testing.h', + 'deps/v8/include/cppgc/trace-trait.h', + 'deps/v8/include/cppgc/type-traits.h', + 'deps/v8/include/cppgc/visitor.h', + # libplatform headers 'deps/v8/include/libplatform/libplatform-export.h', + 'deps/v8/include/libplatform/libplatform.h', 'deps/v8/include/libplatform/v8-tracing.h', - 'deps/v8/include/v8.h', + # v8 headers 'deps/v8/include/v8-array-buffer.h', 'deps/v8/include/v8-callbacks.h', 'deps/v8/include/v8-container.h', 'deps/v8/include/v8-context.h', + 'deps/v8/include/v8-cppgc.h', 'deps/v8/include/v8-data.h', 'deps/v8/include/v8-date.h', 'deps/v8/include/v8-debug.h', @@ -249,6 +295,7 @@ def wanted_v8_headers(files_arg, dest): 'deps/v8/include/v8-version.h', 'deps/v8/include/v8-wasm.h', 'deps/v8/include/v8-weak-callback-info.h', + 'deps/v8/include/v8.h', 'deps/v8/include/v8config.h', ] files_arg = [name for name in files_arg if name in v8_headers]