src: remove dependency on wrapper-descriptor-based cpp heap

As V8 is moving towards built-in CppHeap creation, change the
management so that the automatic CppHeap creation on Node.js's end
is also enforced at Isolate creation time.

1. If embedder uses NewIsolate(), either they use
  IsolateSettings::cpp_heap to specify a CppHeap that will be owned
  by V8, or if it's not configured, Node.js will create a CppHeap
  that will be owned by V8.
2. If the embedder uses SetIsolateUpForNode(),
  IsolateSettings::cpp_heap will be ignored (as V8 has deprecated
  attaching CppHeap post-isolate-creation). The embedders need to
  ensure that the v8::Isolate has a CppHeap attached while it's
  still used by Node.js, preferably using v8::CreateParams.

See https://issues.chromium.org/issues/42203693 for details. In
future version of V8, this CppHeap will be created by V8 if not
provided, and we can remove our own "if no CppHeap provided,
create one" code in NewIsolate().

This also deprecates node::SetCppgcReference() in favor of
v8::Object::Wrap() since the wrapper descriptor is no longer
relevant. It is still kept as a compatibility layer for addons
that need to also work on Node.js versions without
v8::Object::Wrap().

src/api/environment.cc

@@ -19,12 +19,15 @@
 #if HAVE_INSPECTOR
 #include "inspector/worker_inspector.h"  // ParentInspectorHandle
 #endif
+#include "v8-cppgc.h"
 
 namespace node {
 using errors::TryCatchScope;
 using v8::Array;
 using v8::Boolean;
 using v8::Context;
+using v8::CppHeap;
+using v8::CppHeapCreateParams;
 using v8::EscapableHandleScope;
 using v8::Function;
 using v8::FunctionCallbackInfo;
@@ -354,6 +357,14 @@ Isolate* NewIsolate(Isolate::CreateParams* params,
   // so that the isolate can access the platform during initialization.
   platform->RegisterIsolate(isolate, event_loop);
 
+  // Ensure that there is always a CppHeap.
+  if (settings.cpp_heap == nullptr) {
+    params->cpp_heap =
+        CppHeap::Create(platform, CppHeapCreateParams{{}}).release();
+  } else {
+    params->cpp_heap = settings.cpp_heap;
+  }
+
   SetIsolateCreateParamsForNode(params);
   Isolate::Initialize(isolate, *params);
 

src/env-inl.h

@@ -35,6 +35,7 @@
 #include "util-inl.h"
 #include "uv.h"
 #include "v8-cppgc.h"
+#include "v8-sandbox.h"
 #include "v8.h"
 
 #include <cstddef>
@@ -62,31 +63,6 @@ inline uv_loop_t* IsolateData::event_loop() const {
   return event_loop_;
 }
 
-inline void IsolateData::SetCppgcReference(v8::Isolate* isolate,
-                                           v8::Local<v8::Object> 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);
 }

src/env.cc

@@ -40,8 +40,6 @@ 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;
@@ -70,7 +68,6 @@ using v8::TryCatch;
 using v8::Uint32;
 using v8::Undefined;
 using v8::Value;
-using v8::WrapperDescriptor;
 using worker::Worker;
 
 int const ContextEmbedderTag::kNodeContextTag = 0x6e6f64;
@@ -520,6 +517,14 @@ void IsolateData::CreateProperties() {
   CreateEnvProxyTemplate(this);
 }
 
+// Previously, the general convention of the wrappable layout for cppgc in
+// the ecosystem is:
+// [  0  ] -> embedder id
+// [  1  ] -> wrappable instance
+// Now V8 has deprecated this layout-based tracing enablement, embeders
+// should simply use v8::Object::Wrap() and v8::Object::Unwrap(). We preserve
+// this layout only to distinguish internally how the memory of a Node.js
+// wrapper is managed or whether a wrapper is managed by Node.js.
 constexpr uint16_t kDefaultCppGCEmebdderID = 0x90de;
 Mutex IsolateData::isolate_data_mutex_;
 std::unordered_map<uint16_t, std::unique_ptr<PerIsolateWrapperData>>
@@ -538,36 +543,8 @@ IsolateData::IsolateData(Isolate* isolate,
       snapshot_data_(snapshot_data) {
   options_.reset(
       new PerIsolateOptions(*(per_process::cli_options->per_isolate)));
-  v8::CppHeap* cpp_heap = isolate->GetCppHeap();
 
   uint16_t cppgc_id = kDefaultCppGCEmebdderID;
-  if (cpp_heap != nullptr) {
-    // The general convention of the wrappable layout for cppgc in the
-    // ecosystem is:
-    // [  0  ] -> embedder id
-    // [  1  ] -> wrappable instance
-    // If the Isolate includes a CppHeap attached by another embedder,
-    // And if they also use the field 0 for the ID, we DCHECK that
-    // the layout matches our layout, and record the embedder ID for cppgc
-    // to avoid accidentally enabling cppgc on non-cppgc-managed wrappers .
-    v8::WrapperDescriptor descriptor = cpp_heap->wrapper_descriptor();
-    if (descriptor.wrappable_type_index == BaseObject::kEmbedderType) {
-      cppgc_id = descriptor.embedder_id_for_garbage_collected;
-      DCHECK_EQ(descriptor.wrappable_instance_index, BaseObject::kSlot);
-    }
-    // If the CppHeap uses the slot we use to put non-cppgc-traced BaseObject
-    // 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.
   uint16_t non_cppgc_id = cppgc_id + 1;
@@ -594,19 +571,14 @@ IsolateData::IsolateData(Isolate* isolate,
   }
 }
 
-IsolateData::~IsolateData() {
-  if (cpp_heap_ != nullptr) {
-    // The CppHeap must be detached before being terminated.
-    isolate_->DetachCppHeap();
-    cpp_heap_->Terminate();
-  }
-}
+IsolateData::~IsolateData() {}
 
-// Public API
+// Deprecated API, embedders should use v8::Object::Wrap() directly instead.
 void SetCppgcReference(Isolate* isolate,
                        Local<Object> object,
                        void* wrappable) {
-  IsolateData::SetCppgcReference(isolate, object, wrappable);
+  v8::Object::Wrap<v8::CppHeapPointerTag::kDefaultTag>(
+      isolate, object, wrappable);
 }
 
 void IsolateData::MemoryInfo(MemoryTracker* tracker) const {

src/env.h

@@ -67,10 +67,6 @@
 #include <unordered_set>
 #include <vector>
 
-namespace v8 {
-class CppHeap;
-}
-
 namespace node {
 
 namespace shadow_realm {
@@ -165,10 +161,6 @@ 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<v8::Object> object,
-                                       void* wrappable);
-
   inline uv_loop_t* event_loop() const;
   inline MultiIsolatePlatform* platform() const;
   inline const SnapshotData* snapshot_data() const;
@@ -253,7 +245,6 @@ class NODE_EXTERN_PRIVATE IsolateData : public MemoryRetainer {
   const SnapshotData* snapshot_data_;
   std::optional<SnapshotConfig> snapshot_config_;
 
-  std::unique_ptr<v8::CppHeap> cpp_heap_;
   std::shared_ptr<PerIsolateOptions> options_;
   worker::Worker* worker_context_ = nullptr;
   PerIsolateWrapperData* wrapper_data_;

src/node.h

@@ -491,6 +491,21 @@ struct IsolateSettings {
       allow_wasm_code_generation_callback = nullptr;
   v8::ModifyCodeGenerationFromStringsCallback2
       modify_code_generation_from_strings_callback = nullptr;
+
+  // When the settings is passed to NewIsolate():
+  // - If cpp_heap is not nullptr, this CppHeap will be used to create
+  //   the isolate and its ownership will be passed to V8.
+  // - If this is nullptr, Node.js will create a CppHeap that will be
+  //   owned by V8.
+  //
+  // When the settings is passed to SetIsolateUpForNode():
+  // cpp_heap will be ignored. Embedders must ensure that the
+  // v8::Isolate has a CppHeap attached while it's still used by
+  // Node.js, for example using v8::CreateParams.
+  //
+  // See https://issues.chromium.org/issues/42203693. In future version
+  // of V8, this CppHeap will be created by V8 if not provided.
+  v8::CppHeap* cpp_heap = nullptr;
 };
 
 // Represents a startup snapshot blob, e.g. created by passing
@@ -1548,24 +1563,13 @@ 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<v8::Object> object,
-                                   void* wrappable);
+// This is kept as a compatibility layer for addons to wrap cppgc-managed objects
+// on Node.js versions without v8::Object::Wrap(). Addons created to work with
+// only Node.js versions with v8::Object::Wrap() should use that instead.
+NODE_DEPRECATED("Use v8::Object::Wrap()",
+                NODE_EXTERN void SetCppgcReference(v8::Isolate* isolate,
+                                                   v8::Local<v8::Object> object,
+                                                   void* wrappable));
 
 }  // namespace node
 

test/addons/cppgc-object/binding.cc

@@ -1,8 +1,10 @@
+#include <assert.h>
 #include <cppgc/allocation.h>
 #include <cppgc/garbage-collected.h>
 #include <cppgc/heap.h>
 #include <node.h>
 #include <v8-cppgc.h>
+#include <v8-sandbox.h>
 #include <v8.h>
 #include <algorithm>
 
@@ -15,21 +17,17 @@ class CppGCed : public cppgc::GarbageCollected<CppGCed> {
   static void New(const v8::FunctionCallbackInfo<v8::Value>& args) {
     v8::Isolate* isolate = args.GetIsolate();
     v8::Local<v8::Object> js_object = args.This();
-    CppGCed* gc_object = cppgc::MakeGarbageCollected<CppGCed>(
-        isolate->GetCppHeap()->GetAllocationHandle());
+    auto* heap = isolate->GetCppHeap();
+    assert(heap != nullptr);
+    CppGCed* gc_object =
+        cppgc::MakeGarbageCollected<CppGCed>(heap->GetAllocationHandle());
     node::SetCppgcReference(isolate, js_object, gc_object);
     args.GetReturnValue().Set(js_object);
   }
 
   static v8::Local<v8::Function> GetConstructor(
       v8::Local<v8::Context> 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();
   }
 

test/cctest/test_cppgc.cc

@@ -3,16 +3,12 @@
 #include <cppgc/heap.h>
 #include <node.h>
 #include <v8-cppgc.h>
+#include <v8-sandbox.h>
 #include <v8.h>
 #include "node_test_fixture.h"
 
 // This tests that Node.js can work with an existing CppHeap.
 
-// Mimic the Blink layout.
-static int kWrappableTypeIndex = 0;
-static int kWrappableInstanceIndex = 1;
-static uint16_t kEmbedderID = 0x1;
-
 // Mimic a class that does not know about Node.js.
 class CppGCed : public cppgc::GarbageCollected<CppGCed> {
  public:
@@ -23,21 +19,18 @@ class CppGCed : public cppgc::GarbageCollected<CppGCed> {
   static void New(const v8::FunctionCallbackInfo<v8::Value>& args) {
     v8::Isolate* isolate = args.GetIsolate();
     v8::Local<v8::Object> js_object = args.This();
-    CppGCed* gc_object = cppgc::MakeGarbageCollected<CppGCed>(
-        isolate->GetCppHeap()->GetAllocationHandle());
-    js_object->SetAlignedPointerInInternalField(kWrappableTypeIndex,
-                                                &kEmbedderID);
-    js_object->SetAlignedPointerInInternalField(kWrappableInstanceIndex,
-                                                gc_object);
+    auto* heap = isolate->GetCppHeap();
+    CHECK_NOT_NULL(heap);
+    CppGCed* gc_object =
+        cppgc::MakeGarbageCollected<CppGCed>(heap->GetAllocationHandle());
+    node::SetCppgcReference(isolate, js_object, gc_object);
     kConstructCount++;
     args.GetReturnValue().Set(js_object);
   }
 
   static v8::Local<v8::Function> GetConstructor(
       v8::Local<v8::Context> context) {
     auto ft = v8::FunctionTemplate::New(context->GetIsolate(), New);
-    auto ot = ft->InstanceTemplate();
-    ot->SetInternalFieldCount(2);
     return ft->GetFunction(context).ToLocalChecked();
   }
 
@@ -53,18 +46,12 @@ int CppGCed::kDestructCount = 0;
 int CppGCed::kTraceCount = 0;
 
 TEST_F(NodeZeroIsolateTestFixture, ExistingCppHeapTest) {
-  v8::Isolate* isolate =
-      node::NewIsolate(allocator.get(), &current_loop, platform.get());
-
-  // Create and attach the CppHeap before we set up the IsolateData so that
-  // it recognizes the existing heap.
-  std::unique_ptr<v8::CppHeap> cpp_heap = v8::CppHeap::Create(
-      platform.get(),
-      v8::CppHeapCreateParams(
-          {},
-          v8::WrapperDescriptor(
-              kWrappableTypeIndex, kWrappableInstanceIndex, kEmbedderID)));
-  isolate->AttachCppHeap(cpp_heap.get());
+  node::IsolateSettings settings;
+  settings.cpp_heap =
+      v8::CppHeap::Create(platform.get(), v8::CppHeapCreateParams{{}})
+          .release();
+  v8::Isolate* isolate = node::NewIsolate(
+      allocator.get(), &current_loop, platform.get(), nullptr, settings);
 
   // Try creating Context + IsolateData + Environment.
   {
@@ -107,11 +94,6 @@ TEST_F(NodeZeroIsolateTestFixture, ExistingCppHeapTest) {
     }
 
     platform->DrainTasks(isolate);
-
-    // Cleanup.
-    isolate->DetachCppHeap();
-    cpp_heap->Terminate();
-    platform->DrainTasks(isolate);
   }
 
   platform->UnregisterIsolate(isolate);