diff --git a/src/node_contextify.cc b/src/node_contextify.cc index 77d35675827c67..75398415217f88 100644 --- a/src/node_contextify.cc +++ b/src/node_contextify.cc @@ -118,8 +118,9 @@ Local Uint32ToName(Local context, uint32_t index) { } // anonymous namespace -BaseObjectPtr ContextifyContext::New( - Environment* env, Local sandbox_obj, ContextOptions* options) { +ContextifyContext* ContextifyContext::New(Environment* env, + Local sandbox_obj, + ContextOptions* options) { Local object_template; HandleScope scope(env->isolate()); CHECK_IMPLIES(sandbox_obj.IsEmpty(), options->vanilla); @@ -140,21 +141,25 @@ BaseObjectPtr 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(); + 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 wrapper, Local 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. @@ -162,21 +167,20 @@ ContextifyContext::ContextifyContext(Environment* env, 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_)); + env->UntrackContext(context()); context_.Reset(); } +ContextifyContext::~ContextifyContext() { + Clean(); +} + void ContextifyContext::InitializeGlobalTemplates(IsolateData* isolate_data) { DCHECK(isolate_data->contextify_wrapper_template().IsEmpty()); Local global_func_template = @@ -251,11 +255,10 @@ MaybeLocal ContextifyContext::CreateV8Context( return scope.Escape(ctx); } -BaseObjectPtr ContextifyContext::New( - Local v8_context, - Environment* env, - Local sandbox_obj, - ContextOptions* options) { +ContextifyContext* ContextifyContext::New(Local v8_context, + Environment* env, + Local 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 @@ -263,7 +266,7 @@ BaseObjectPtr ContextifyContext::New( // things down significantly and they are only needed in rare occasions // in the vm contexts. if (InitializeContextRuntime(v8_context).IsNothing()) { - return BaseObjectPtr(); + return nullptr; } Local main_context = env->context(); @@ -300,7 +303,7 @@ BaseObjectPtr ContextifyContext::New( info.origin = *origin_val; } - BaseObjectPtr result; + ContextifyContext* result; Local wrapper; { Context::Scope context_scope(v8_context); @@ -315,7 +318,7 @@ BaseObjectPtr ContextifyContext::New( ctor_name, static_cast(v8::DontEnum)) .IsNothing()) { - return BaseObjectPtr(); + return nullptr; } } @@ -328,7 +331,7 @@ BaseObjectPtr ContextifyContext::New( env->host_defined_option_symbol(), options->host_defined_options_id) .IsNothing()) { - return BaseObjectPtr(); + return nullptr; } env->AssignToContext(v8_context, nullptr, info); @@ -336,13 +339,15 @@ BaseObjectPtr ContextifyContext::New( if (!env->contextify_wrapper_template() ->NewInstance(v8_context) .ToLocal(&wrapper)) { - return BaseObjectPtr(); + return nullptr; } - result = - MakeBaseObject(env, wrapper, v8_context, options); - // The only strong reference to the wrapper will come from the sandbox. - result->MakeWeak(); + result = cppgc::MakeGarbageCollected( + env->isolate()->GetCppHeap()->GetAllocationHandle(), + env, + wrapper, + v8_context, + options); } Local wrapper_holder = @@ -352,7 +357,7 @@ BaseObjectPtr ContextifyContext::New( ->SetPrivate( v8_context, env->contextify_context_private_symbol(), wrapper) .IsNothing()) { - return BaseObjectPtr(); + return nullptr; } // Assign host_defined_options_id to the sandbox object or the global object @@ -364,7 +369,7 @@ BaseObjectPtr ContextifyContext::New( env->host_defined_option_symbol(), options->host_defined_options_id) .IsNothing()) { - return BaseObjectPtr(); + return nullptr; } return result; } @@ -438,7 +443,7 @@ void ContextifyContext::MakeContext(const FunctionCallbackInfo& args) { options.host_defined_options_id = args[6].As(); TryCatchScope try_catch(env); - BaseObjectPtr context_ptr = + ContextifyContext* context_ptr = ContextifyContext::New(env, sandbox, &options); if (try_catch.HasCaught()) { @@ -469,6 +474,10 @@ ContextifyContext* ContextifyContext::ContextFromContextifiedSandbox( template ContextifyContext* ContextifyContext::Get(const PropertyCallbackInfo& 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 index d67968406d7b74..c350a2e91b59a2 100644 --- a/src/node_contextify.h +++ b/src/node_contextify.h @@ -23,17 +23,66 @@ 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 acts as a shortcut 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 wrapper, v8::Local 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 CreateV8Context( v8::Isolate* isolate, @@ -48,7 +97,7 @@ class ContextifyContext : public BaseObject { Environment* env, const v8::Local& wrapper_holder); inline v8::Local context() const { - return PersistentToLocal::Default(env()->isolate(), context_); + return context_.Get(env()->isolate()); } inline v8::Local global_proxy() const { @@ -75,14 +124,14 @@ class ContextifyContext : public BaseObject { static void InitializeGlobalTemplates(IsolateData* isolate_data); private: - static BaseObjectPtr New(Environment* env, - v8::Local sandbox_obj, - ContextOptions* options); + static ContextifyContext* New(Environment* env, + v8::Local sandbox_obj, + ContextOptions* options); // Initialize a context created from CreateV8Context() - static BaseObjectPtr New(v8::Local ctx, - Environment* env, - v8::Local sandbox_obj, - ContextOptions* options); + static ContextifyContext* New(v8::Local ctx, + Environment* env, + v8::Local sandbox_obj, + ContextOptions* options); static bool IsStillInitializing(const ContextifyContext* ctx); static void MakeContext(const v8::FunctionCallbackInfo& args); @@ -140,7 +189,7 @@ class ContextifyContext : public BaseObject { static void IndexedPropertyEnumeratorCallback( const v8::PropertyCallbackInfo& args); - v8::Global context_; + v8::TracedReference context_; std::unique_ptr microtask_queue_; };