Skip to content

Commit

Permalink
src: create BaseObject with node::Realm
Browse files Browse the repository at this point in the history
BaseObject is a wrapper around JS objects. These objects should be
created in a node::Realm and destroyed when their associated realm is
cleaning up.

PR-URL: #44348
Refs: #42528
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
  • Loading branch information
legendecas authored and danielleadams committed Jan 3, 2023
1 parent a7f3bc0 commit 7b992cc
Show file tree
Hide file tree
Showing 15 changed files with 381 additions and 288 deletions.
1 change: 1 addition & 0 deletions node.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -477,6 +477,7 @@
'src/api/hooks.cc',
'src/api/utils.cc',
'src/async_wrap.cc',
'src/base_object.cc',
'src/cares_wrap.cc',
'src/cleanup_queue.cc',
'src/connect_wrap.cc',
Expand Down
2 changes: 1 addition & 1 deletion src/api/embed_helpers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ Maybe<int> SpinEventLoop(Environment* env) {
env->set_snapshot_serialize_callback(Local<Function>());

env->PrintInfoForSnapshotIfDebug();
env->VerifyNoStrongBaseObjects();
env->ForEachRealm([](Realm* realm) { realm->VerifyNoStrongBaseObjects(); });
return EmitProcessExit(env);
}

Expand Down
9 changes: 8 additions & 1 deletion src/base_object-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,9 @@

namespace node {

BaseObject::BaseObject(Environment* env, v8::Local<v8::Object> object)
: BaseObject(env->principal_realm(), object) {}

// static
v8::Local<v8::FunctionTemplate> BaseObject::GetConstructorTemplate(
Environment* env) {
Expand Down Expand Up @@ -63,7 +66,11 @@ v8::Local<v8::Object> BaseObject::object(v8::Isolate* isolate) const {
}

Environment* BaseObject::env() const {
return env_;
return realm_->env();
}

Realm* BaseObject::realm() const {
return realm_;
}

BaseObject* BaseObject::FromJSObject(v8::Local<v8::Value> value) {
Expand Down
164 changes: 164 additions & 0 deletions src/base_object.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,164 @@
#include "base_object.h"
#include "env-inl.h"
#include "node_realm-inl.h"

namespace node {

using v8::FunctionCallbackInfo;
using v8::FunctionTemplate;
using v8::HandleScope;
using v8::Local;
using v8::Object;
using v8::Value;
using v8::WeakCallbackInfo;
using v8::WeakCallbackType;

BaseObject::BaseObject(Realm* realm, Local<Object> object)
: persistent_handle_(realm->isolate(), object), realm_(realm) {
CHECK_EQ(false, object.IsEmpty());
CHECK_GE(object->InternalFieldCount(), BaseObject::kInternalFieldCount);
object->SetAlignedPointerInInternalField(BaseObject::kEmbedderType,
&kNodeEmbedderId);
object->SetAlignedPointerInInternalField(BaseObject::kSlot,
static_cast<void*>(this));
realm->AddCleanupHook(DeleteMe, static_cast<void*>(this));
realm->modify_base_object_count(1);
}

BaseObject::~BaseObject() {
realm()->modify_base_object_count(-1);
realm()->RemoveCleanupHook(DeleteMe, static_cast<void*>(this));

if (UNLIKELY(has_pointer_data())) {
PointerData* metadata = pointer_data();
CHECK_EQ(metadata->strong_ptr_count, 0);
metadata->self = nullptr;
if (metadata->weak_ptr_count == 0) delete metadata;
}

if (persistent_handle_.IsEmpty()) {
// This most likely happened because the weak callback below cleared it.
return;
}

{
HandleScope handle_scope(realm()->isolate());
object()->SetAlignedPointerInInternalField(BaseObject::kSlot, nullptr);
}
}

void BaseObject::MakeWeak() {
if (has_pointer_data()) {
pointer_data()->wants_weak_jsobj = true;
if (pointer_data()->strong_ptr_count > 0) return;
}

persistent_handle_.SetWeak(
this,
[](const WeakCallbackInfo<BaseObject>& data) {
BaseObject* obj = data.GetParameter();
// Clear the persistent handle so that ~BaseObject() doesn't attempt
// to mess with internal fields, since the JS object may have
// transitioned into an invalid state.
// Refs: https://github.com/nodejs/node/issues/18897
obj->persistent_handle_.Reset();
CHECK_IMPLIES(obj->has_pointer_data(),
obj->pointer_data()->strong_ptr_count == 0);
obj->OnGCCollect();
},
WeakCallbackType::kParameter);
}

// This just has to be different from the Chromium ones:
// https://source.chromium.org/chromium/chromium/src/+/main:gin/public/gin_embedders.h;l=18-23;drc=5a758a97032f0b656c3c36a3497560762495501a
// Otherwise, when Node is loaded in an isolate which uses cppgc, cppgc will
// misinterpret the data stored in the embedder fields and try to garbage
// collect them.
uint16_t kNodeEmbedderId = 0x90de;

void BaseObject::LazilyInitializedJSTemplateConstructor(
const FunctionCallbackInfo<Value>& args) {
DCHECK(args.IsConstructCall());
CHECK_GE(args.This()->InternalFieldCount(), BaseObject::kInternalFieldCount);
args.This()->SetAlignedPointerInInternalField(BaseObject::kEmbedderType,
&kNodeEmbedderId);
args.This()->SetAlignedPointerInInternalField(BaseObject::kSlot, nullptr);
}

Local<FunctionTemplate> BaseObject::MakeLazilyInitializedJSTemplate(
Environment* env) {
Local<FunctionTemplate> t = NewFunctionTemplate(
env->isolate(), LazilyInitializedJSTemplateConstructor);
t->Inherit(BaseObject::GetConstructorTemplate(env));
t->InstanceTemplate()->SetInternalFieldCount(BaseObject::kInternalFieldCount);
return t;
}

BaseObject::PointerData* BaseObject::pointer_data() {
if (!has_pointer_data()) {
PointerData* metadata = new PointerData();
metadata->wants_weak_jsobj = persistent_handle_.IsWeak();
metadata->self = this;
pointer_data_ = metadata;
}
CHECK(has_pointer_data());
return pointer_data_;
}

void BaseObject::decrease_refcount() {
CHECK(has_pointer_data());
PointerData* metadata = pointer_data();
CHECK_GT(metadata->strong_ptr_count, 0);
unsigned int new_refcount = --metadata->strong_ptr_count;
if (new_refcount == 0) {
if (metadata->is_detached) {
OnGCCollect();
} else if (metadata->wants_weak_jsobj && !persistent_handle_.IsEmpty()) {
MakeWeak();
}
}
}

void BaseObject::increase_refcount() {
unsigned int prev_refcount = pointer_data()->strong_ptr_count++;
if (prev_refcount == 0 && !persistent_handle_.IsEmpty())
persistent_handle_.ClearWeak();
}

void BaseObject::DeleteMe(void* data) {
BaseObject* self = static_cast<BaseObject*>(data);
if (self->has_pointer_data() && self->pointer_data()->strong_ptr_count > 0) {
return self->Detach();
}
delete self;
}

bool BaseObject::IsDoneInitializing() const {
return true;
}

Local<Object> BaseObject::WrappedObject() const {
return object();
}

bool BaseObject::IsRootNode() const {
return !persistent_handle_.IsWeak();
}

Local<FunctionTemplate> BaseObject::GetConstructorTemplate(
IsolateData* isolate_data) {
Local<FunctionTemplate> tmpl = isolate_data->base_object_ctor_template();
if (tmpl.IsEmpty()) {
tmpl = NewFunctionTemplate(isolate_data->isolate(), nullptr);
tmpl->SetClassName(
FIXED_ONE_BYTE_STRING(isolate_data->isolate(), "BaseObject"));
isolate_data->set_base_object_ctor_template(tmpl);
}
return tmpl;
}

bool BaseObject::IsNotIndicativeOfMemoryLeakAtExit() const {
return IsWeakOrDetached();
}

} // namespace node
10 changes: 8 additions & 2 deletions src/base_object.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ namespace node {

class Environment;
class IsolateData;
class Realm;
template <typename T, bool kIsWeak>
class BaseObjectPtrImpl;

Expand All @@ -47,7 +48,10 @@ class BaseObject : public MemoryRetainer {

// Associates this object with `object`. It uses the 1st internal field for
// that, and in particular aborts if there is no such field.
BaseObject(Environment* env, v8::Local<v8::Object> object);
// This is the designated constructor.
BaseObject(Realm* realm, v8::Local<v8::Object> object);
// Convenient constructor for constructing BaseObject in the principal realm.
inline BaseObject(Environment* env, v8::Local<v8::Object> object);
~BaseObject() override;

BaseObject() = delete;
Expand All @@ -63,6 +67,7 @@ class BaseObject : public MemoryRetainer {
inline v8::Global<v8::Object>& persistent();

inline Environment* env() const;
inline Realm* realm() const;

// Get a BaseObject* pointer, or subclass pointer, for the JS object that
// was also passed to the `BaseObject()` constructor initially.
Expand Down Expand Up @@ -93,6 +98,7 @@ class BaseObject : public MemoryRetainer {
// Utility to create a FunctionTemplate with one internal field (used for
// the `BaseObject*` pointer) and a constructor that initializes that field
// to `nullptr`.
// TODO(legendecas): Disentangle template with env.
static v8::Local<v8::FunctionTemplate> MakeLazilyInitializedJSTemplate(
Environment* env);

Expand Down Expand Up @@ -215,7 +221,7 @@ class BaseObject : public MemoryRetainer {
void decrease_refcount();
void increase_refcount();

Environment* env_;
Realm* realm_;
PointerData* pointer_data_ = nullptr;
};

Expand Down
27 changes: 6 additions & 21 deletions src/env-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -745,6 +745,12 @@ inline IsolateData* Environment::isolate_data() const {
return isolate_data_;
}

template <typename T>
inline void Environment::ForEachRealm(T&& iterator) const {
// TODO(legendecas): iterate over more realms bound to the environment.
iterator(principal_realm());
}

inline void Environment::ThrowError(const char* errmsg) {
ThrowError(v8::Exception::Error, errmsg);
}
Expand Down Expand Up @@ -789,27 +795,6 @@ void Environment::RemoveCleanupHook(CleanupQueue::Callback fn, void* arg) {
cleanup_queue_.Remove(fn, arg);
}

template <typename T>
void Environment::ForEachBaseObject(T&& iterator) {
cleanup_queue_.ForEachBaseObject(std::forward<T>(iterator));
}

void Environment::modify_base_object_count(int64_t delta) {
base_object_count_ += delta;
}

int64_t Environment::base_object_count() const {
return base_object_count_;
}

inline void Environment::set_base_object_created_by_bootstrap(int64_t count) {
base_object_created_by_bootstrap_ = base_object_count_;
}

int64_t Environment::base_object_created_after_bootstrap() const {
return base_object_count_ - base_object_created_by_bootstrap_;
}

void Environment::set_main_utf16(std::unique_ptr<v8::String::Value> str) {
CHECK(!main_utf16_);
main_utf16_ = std::move(str);
Expand Down
Loading

0 comments on commit 7b992cc

Please sign in to comment.