src: use cppgc to manage ContextifyContext

joyeecheung · joyeecheung · commit bcb919d2d97f · 2025-01-10T17:17:31.000+01:00
This simplifies the memory management of ContextifyContext,
making all references visible to V8.
diff --git a/src/env.cc b/src/env.cc
@@ -230,9 +230,13 @@ void Environment::TrackContext(Local<Context> context) {
   contexts_[id].SetWeak();
 }
 
+void Environment::PurgeTrackedEmptyContexts() {
+  std::erase_if(contexts_, [&](auto&& el) { return el.IsEmpty(); });
+}
+
 void Environment::UntrackContext(Local<Context> context) {
   HandleScope handle_scope(isolate_);
-  std::erase_if(contexts_, [&](auto&& el) { return el.IsEmpty(); });
+  PurgeTrackedEmptyContexts();
   for (auto it = contexts_.begin(); it != contexts_.end(); it++) {
     if (Local<Context> saved_context = PersistentToLocal::Weak(isolate_, *it);
         saved_context == context) {
diff --git a/src/env.h b/src/env.h
@@ -703,6 +703,8 @@ class Environment final : public MemoryRetainer {
                        Realm* realm,
                        const ContextInfo& info);
   void UnassignFromContext(v8::Local<v8::Context> context);
+  void PurgeTrackedEmptyContexts();
+
   void TrackShadowRealm(shadow_realm::ShadowRealm* realm);
   void UntrackShadowRealm(shadow_realm::ShadowRealm* realm);
 
diff --git a/src/node_contextify.cc b/src/node_contextify.cc
@@ -118,8 +118,9 @@ Local<Name> Uint32ToName(Local<Context> context, uint32_t index) {
 
 }  // anonymous namespace
 
-BaseObjectPtr<ContextifyContext> ContextifyContext::New(
-    Environment* env, Local<Object> sandbox_obj, ContextOptions* options) {
+ContextifyContext* ContextifyContext::New(Environment* env,
+                                          Local<Object> sandbox_obj,
+                                          ContextOptions* options) {
   Local<ObjectTemplate> object_template;
   HandleScope scope(env->isolate());
   CHECK_IMPLIES(sandbox_obj.IsEmpty(), options->vanilla);
@@ -140,43 +141,56 @@ BaseObjectPtr<ContextifyContext> ContextifyContext::New(
   if (!(CreateV8Context(env->isolate(), object_template, snapshot_data, queue)
             .ToLocal(&v8_context))) {
     // Allocation failure, maximum call stack size reached, termination, etc.
-    return BaseObjectPtr<ContextifyContext>();
+    return nullptr;
   }
   return New(v8_context, env, sandbox_obj, options);
 }
 
-void ContextifyContext::MemoryInfo(MemoryTracker* tracker) const {}
+void ContextifyContext::Trace(cppgc::Visitor* visitor) const {
+  CppgcMixin::Trace(visitor);
+  visitor->Trace(context_);
+}
 
 ContextifyContext::ContextifyContext(Environment* env,
                                      Local<Object> wrapper,
                                      Local<Context> v8_context,
                                      ContextOptions* options)
-    : BaseObject(env, wrapper),
-      microtask_queue_(options->own_microtask_queue
+    : microtask_queue_(options->own_microtask_queue
                            ? options->own_microtask_queue.release()
                            : nullptr) {
+  CppgcMixin::Wrap(this, env, wrapper);
+
   context_.Reset(env->isolate(), v8_context);
   // This should only be done after the initial initializations of the context
   // global object is finished.
   DCHECK_NULL(v8_context->GetAlignedPointerFromEmbedderData(
       ContextEmbedderIndex::kContextifyContext));
   v8_context->SetAlignedPointerInEmbedderData(
       ContextEmbedderIndex::kContextifyContext, this);
-  // It's okay to make this reference weak - V8 would create an internal
-  // reference to this context via the constructor of the wrapper.
-  // As long as the wrapper is alive, it's constructor is alive, and so
-  // is the context.
-  context_.SetWeak();
 }
 
-ContextifyContext::~ContextifyContext() {
-  Isolate* isolate = env()->isolate();
+void ContextifyContext::CleanEnvResource(Environment* env) {
+  Isolate* isolate = env->isolate();
   HandleScope scope(isolate);
 
-  env()->UnassignFromContext(PersistentToLocal::Weak(isolate, context_));
+  // PurgeTrackedEmptyContexts() is all we need because when the
+  // wrapper is unreachable, the v8::Context is also already going
+  // away and there's no point to reset its internal pointers.
+  // In the context list, the global handle to the original
+  // context should have already been empty and will be removed
+  // in PurgeTrackedEmptyContexts().
+  // TODO(joyeecheung): consider just using a default destructor.
+  // We don't even need to purge the empty contexts in the
+  // destructors if  TrackContext() purges them, restricting the
+  // amount of empty contexts in the list.
+  env->PurgeTrackedEmptyContexts();
   context_.Reset();
 }
 
+ContextifyContext::~ContextifyContext() {
+  Clean();
+}
+
 void ContextifyContext::InitializeGlobalTemplates(IsolateData* isolate_data) {
   DCHECK(isolate_data->contextify_wrapper_template().IsEmpty());
   Local<FunctionTemplate> global_func_template =
@@ -251,19 +265,18 @@ MaybeLocal<Context> ContextifyContext::CreateV8Context(
   return scope.Escape(ctx);
 }
 
-BaseObjectPtr<ContextifyContext> ContextifyContext::New(
-    Local<Context> v8_context,
-    Environment* env,
-    Local<Object> sandbox_obj,
-    ContextOptions* options) {
+ContextifyContext* ContextifyContext::New(Local<Context> v8_context,
+                                          Environment* env,
+                                          Local<Object> sandbox_obj,
+                                          ContextOptions* options) {
   HandleScope scope(env->isolate());
   CHECK_IMPLIES(sandbox_obj.IsEmpty(), options->vanilla);
   // This only initializes part of the context. The primordials are
   // only initialized when needed because even deserializing them slows
   // things down significantly and they are only needed in rare occasions
   // in the vm contexts.
   if (InitializeContextRuntime(v8_context).IsNothing()) {
-    return BaseObjectPtr<ContextifyContext>();
+    return nullptr;
   }
 
   Local<Context> main_context = env->context();
@@ -300,7 +313,7 @@ BaseObjectPtr<ContextifyContext> ContextifyContext::New(
     info.origin = *origin_val;
   }
 
-  BaseObjectPtr<ContextifyContext> result;
+  ContextifyContext* result;
   Local<Object> wrapper;
   {
     Context::Scope context_scope(v8_context);
@@ -315,7 +328,7 @@ BaseObjectPtr<ContextifyContext> ContextifyContext::New(
                   ctor_name,
                   static_cast<v8::PropertyAttribute>(v8::DontEnum))
               .IsNothing()) {
-        return BaseObjectPtr<ContextifyContext>();
+        return nullptr;
       }
     }
 
@@ -328,21 +341,23 @@ BaseObjectPtr<ContextifyContext> ContextifyContext::New(
                          env->host_defined_option_symbol(),
                          options->host_defined_options_id)
             .IsNothing()) {
-      return BaseObjectPtr<ContextifyContext>();
+      return nullptr;
     }
 
     env->AssignToContext(v8_context, nullptr, info);
 
     if (!env->contextify_wrapper_template()
              ->NewInstance(v8_context)
              .ToLocal(&wrapper)) {
-      return BaseObjectPtr<ContextifyContext>();
+      return nullptr;
     }
 
-    result =
-        MakeBaseObject<ContextifyContext>(env, wrapper, v8_context, options);
-    // The only strong reference to the wrapper will come from the sandbox.
-    result->MakeWeak();
+    result = cppgc::MakeGarbageCollected<ContextifyContext>(
+        env->isolate()->GetCppHeap()->GetAllocationHandle(),
+        env,
+        wrapper,
+        v8_context,
+        options);
   }
 
   Local<Object> wrapper_holder =
@@ -352,7 +367,7 @@ BaseObjectPtr<ContextifyContext> ContextifyContext::New(
           ->SetPrivate(
               v8_context, env->contextify_context_private_symbol(), wrapper)
           .IsNothing()) {
-    return BaseObjectPtr<ContextifyContext>();
+    return nullptr;
   }
 
   // Assign host_defined_options_id to the sandbox object or the global object
@@ -364,7 +379,7 @@ BaseObjectPtr<ContextifyContext> ContextifyContext::New(
                        env->host_defined_option_symbol(),
                        options->host_defined_options_id)
           .IsNothing()) {
-    return BaseObjectPtr<ContextifyContext>();
+    return nullptr;
   }
   return result;
 }
@@ -438,7 +453,7 @@ void ContextifyContext::MakeContext(const FunctionCallbackInfo<Value>& args) {
   options.host_defined_options_id = args[6].As<Symbol>();
 
   TryCatchScope try_catch(env);
-  BaseObjectPtr<ContextifyContext> context_ptr =
+  ContextifyContext* context_ptr =
       ContextifyContext::New(env, sandbox, &options);
 
   if (try_catch.HasCaught()) {
@@ -469,6 +484,10 @@ ContextifyContext* ContextifyContext::ContextFromContextifiedSandbox(
 
 template <typename T>
 ContextifyContext* ContextifyContext::Get(const PropertyCallbackInfo<T>& args) {
+  // TODO(joyeecheung): it should be fine to simply use
+  // args.GetIsolate()->GetCurrentContext() and take the pointer at
+  // ContextEmbedderIndex::kContextifyContext, as V8 is supposed to
+  // push the creation context before invoking these callbacks.
   return Get(args.This());
 }
 
diff --git a/src/node_contextify.h b/src/node_contextify.h
@@ -4,6 +4,7 @@
 #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
 
 #include "base_object-inl.h"
+#include "cppgc/prefinalizer.h"
 #include "cppgc_helpers.h"
 #include "node_context_data.h"
 #include "node_errors.h"
@@ -23,17 +24,67 @@ struct ContextOptions {
   bool vanilla = false;
 };
 
-class ContextifyContext : public BaseObject {
+/**
+ * The memory management of a vm context is as follows:
+ *
+ *                                                          user code
+ *                                                              │
+ *                          As global proxy or                  ▼
+ *     ┌──────────────┐  kSandboxObject embedder data   ┌────────────────┐
+ * ┌─► │  V8 Context  │────────────────────────────────►│ Wrapper holder │
+ * │   └──────────────┘                                 └───────┬────────┘
+ * │         ▲  Object constructor/creation context             │
+ * │         │                                                  │
+ * │  ┌──────┴────────────┐  contextify_context_private_symbol  │
+ * │  │ ContextifyContext │◄────────────────────────────────────┘
+ * │  │   JS Wrapper      │◄──────────► ┌─────────────────────────┐
+ * │  └───────────────────┘  cppgc      │ node::ContextifyContext │
+ * │                                    │       C++ Object        │
+ * └──────────────────────────────────► └─────────────────────────┘
+ *     v8::TracedReference / ContextEmbedderIndex::kContextifyContext
+ *
+ * There are two possibilities for the "wrapper holder":
+ *
+ * 1. When vm.constants.DONT_CONTEXTIFY is used, the wrapper holder is the V8
+ *    context's global proxy object
+ * 2. Otherwise it's the arbitrary "sandbox object" that users pass into
+ *    vm.createContext() or a new empty object created internally if they pass
+ *    undefined.
+ *
+ * In 2, the global object of the new V8 context is created using
+ * global_object_template with interceptors that perform any requested
+ * operations on the global object in the context first on the sandbox object
+ * living outside of the new context, then fall back to the global proxy of the
+ * new context.
+ *
+ * It's critical for the user-accessible wrapper holder to keep the
+ * ContextifyContext wrapper alive via contextify_context_private_symbol
+ * so that the V8 context is always available to the user while they still
+ * hold the vm "context" object alive.
+ *
+ * It's also critical for the V8 context to keep the wrapper holder
+ * (specifically, the "sandbox object") as well as the node::ContextifyContext
+ * C++ object alive, so that when the code runs inside the object and accesses
+ * the global object, the interceptors can still access the "sandbox object"
+ * as well as and perform operations
+ * on them, even if users already relinquish access to the outer
+ * "sandbox object".
+ *
+ * The v8::TracedReference and the ContextEmbedderIndex::kContextifyContext
+ * slot in the context act as shortcuts from the node::ContextifyContext
+ * C++ object to the V8 context.
+ */
+class ContextifyContext final : CPPGC_MIXIN(ContextifyContext) {
  public:
+  SET_CPPGC_NAME(ContextifyContext)
+  void Trace(cppgc::Visitor* visitor) const final;
+  void CleanEnvResource(Environment* env) override;
+
   ContextifyContext(Environment* env,
                     v8::Local<v8::Object> wrapper,
                     v8::Local<v8::Context> v8_context,
                     ContextOptions* options);
-  ~ContextifyContext();
-
-  void MemoryInfo(MemoryTracker* tracker) const override;
-  SET_MEMORY_INFO_NAME(ContextifyContext)
-  SET_SELF_SIZE(ContextifyContext)
+  ~ContextifyContext() override;
 
   static v8::MaybeLocal<v8::Context> CreateV8Context(
       v8::Isolate* isolate,
@@ -48,7 +99,7 @@ class ContextifyContext : public BaseObject {
       Environment* env, const v8::Local<v8::Object>& wrapper_holder);
 
   inline v8::Local<v8::Context> context() const {
-    return PersistentToLocal::Default(env()->isolate(), context_);
+    return context_.Get(env()->isolate());
   }
 
   inline v8::Local<v8::Object> global_proxy() const {
@@ -75,14 +126,17 @@ class ContextifyContext : public BaseObject {
   static void InitializeGlobalTemplates(IsolateData* isolate_data);
 
  private:
-  static BaseObjectPtr<ContextifyContext> New(Environment* env,
-                                              v8::Local<v8::Object> sandbox_obj,
-                                              ContextOptions* options);
+  // FIXME(joyeecheung): this crashes at PrefinalizerRegistration.
+  // CPPGC_USING_PRE_FINALIZER(ContextifyContext, Clean);
+
+  static ContextifyContext* New(Environment* env,
+                                v8::Local<v8::Object> sandbox_obj,
+                                ContextOptions* options);
   // Initialize a context created from CreateV8Context()
-  static BaseObjectPtr<ContextifyContext> New(v8::Local<v8::Context> ctx,
-                                              Environment* env,
-                                              v8::Local<v8::Object> sandbox_obj,
-                                              ContextOptions* options);
+  static ContextifyContext* New(v8::Local<v8::Context> ctx,
+                                Environment* env,
+                                v8::Local<v8::Object> sandbox_obj,
+                                ContextOptions* options);
 
   static bool IsStillInitializing(const ContextifyContext* ctx);
   static void MakeContext(const v8::FunctionCallbackInfo<v8::Value>& args);
@@ -140,7 +194,7 @@ class ContextifyContext : public BaseObject {
   static void IndexedPropertyEnumeratorCallback(
       const v8::PropertyCallbackInfo<v8::Array>& args);
 
-  v8::Global<v8::Context> context_;
+  v8::TracedReference<v8::Context> context_;
   std::unique_ptr<v8::MicrotaskQueue> microtask_queue_;
 };
 
