Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 7174654

Browse filesBrowse files
legendecaslegendecas
committed
src: create BaseObject with node::Realm
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>
1 parent 9795976 commit 7174654
Copy full SHA for 7174654
Expand file treeCollapse file tree

15 files changed

+381
-288
lines changed
Open diff view settings
Collapse file

‎node.gyp‎

Copy file name to clipboardExpand all lines: node.gyp
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -465,6 +465,7 @@
465465
'src/api/hooks.cc',
466466
'src/api/utils.cc',
467467
'src/async_wrap.cc',
468+
'src/base_object.cc',
468469
'src/cares_wrap.cc',
469470
'src/cleanup_queue.cc',
470471
'src/connect_wrap.cc',
Collapse file

‎src/api/embed_helpers.cc‎

Copy file name to clipboardExpand all lines: src/api/embed_helpers.cc
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ Maybe<int> SpinEventLoop(Environment* env) {
6868
env->set_snapshot_serialize_callback(Local<Function>());
6969

7070
env->PrintInfoForSnapshotIfDebug();
71-
env->VerifyNoStrongBaseObjects();
71+
env->ForEachRealm([](Realm* realm) { realm->VerifyNoStrongBaseObjects(); });
7272
return EmitProcessExit(env);
7373
}
7474

Collapse file

‎src/base_object-inl.h‎

Copy file name to clipboardExpand all lines: src/base_object-inl.h
+8-1Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,9 @@
3232

3333
namespace node {
3434

35+
BaseObject::BaseObject(Environment* env, v8::Local<v8::Object> object)
36+
: BaseObject(env->principal_realm(), object) {}
37+
3538
// static
3639
v8::Local<v8::FunctionTemplate> BaseObject::GetConstructorTemplate(
3740
Environment* env) {
@@ -63,7 +66,11 @@ v8::Local<v8::Object> BaseObject::object(v8::Isolate* isolate) const {
6366
}
6467

6568
Environment* BaseObject::env() const {
66-
return env_;
69+
return realm_->env();
70+
}
71+
72+
Realm* BaseObject::realm() const {
73+
return realm_;
6774
}
6875

6976
BaseObject* BaseObject::FromJSObject(v8::Local<v8::Value> value) {
Collapse file

‎src/base_object.cc‎

Copy file name to clipboard
+164Lines changed: 164 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,164 @@
1+
#include "base_object.h"
2+
#include "env-inl.h"
3+
#include "node_realm-inl.h"
4+
5+
namespace node {
6+
7+
using v8::FunctionCallbackInfo;
8+
using v8::FunctionTemplate;
9+
using v8::HandleScope;
10+
using v8::Local;
11+
using v8::Object;
12+
using v8::Value;
13+
using v8::WeakCallbackInfo;
14+
using v8::WeakCallbackType;
15+
16+
BaseObject::BaseObject(Realm* realm, Local<Object> object)
17+
: persistent_handle_(realm->isolate(), object), realm_(realm) {
18+
CHECK_EQ(false, object.IsEmpty());
19+
CHECK_GE(object->InternalFieldCount(), BaseObject::kInternalFieldCount);
20+
object->SetAlignedPointerInInternalField(BaseObject::kEmbedderType,
21+
&kNodeEmbedderId);
22+
object->SetAlignedPointerInInternalField(BaseObject::kSlot,
23+
static_cast<void*>(this));
24+
realm->AddCleanupHook(DeleteMe, static_cast<void*>(this));
25+
realm->modify_base_object_count(1);
26+
}
27+
28+
BaseObject::~BaseObject() {
29+
realm()->modify_base_object_count(-1);
30+
realm()->RemoveCleanupHook(DeleteMe, static_cast<void*>(this));
31+
32+
if (UNLIKELY(has_pointer_data())) {
33+
PointerData* metadata = pointer_data();
34+
CHECK_EQ(metadata->strong_ptr_count, 0);
35+
metadata->self = nullptr;
36+
if (metadata->weak_ptr_count == 0) delete metadata;
37+
}
38+
39+
if (persistent_handle_.IsEmpty()) {
40+
// This most likely happened because the weak callback below cleared it.
41+
return;
42+
}
43+
44+
{
45+
HandleScope handle_scope(realm()->isolate());
46+
object()->SetAlignedPointerInInternalField(BaseObject::kSlot, nullptr);
47+
}
48+
}
49+
50+
void BaseObject::MakeWeak() {
51+
if (has_pointer_data()) {
52+
pointer_data()->wants_weak_jsobj = true;
53+
if (pointer_data()->strong_ptr_count > 0) return;
54+
}
55+
56+
persistent_handle_.SetWeak(
57+
this,
58+
[](const WeakCallbackInfo<BaseObject>& data) {
59+
BaseObject* obj = data.GetParameter();
60+
// Clear the persistent handle so that ~BaseObject() doesn't attempt
61+
// to mess with internal fields, since the JS object may have
62+
// transitioned into an invalid state.
63+
// Refs: https://github.com/nodejs/node/issues/18897
64+
obj->persistent_handle_.Reset();
65+
CHECK_IMPLIES(obj->has_pointer_data(),
66+
obj->pointer_data()->strong_ptr_count == 0);
67+
obj->OnGCCollect();
68+
},
69+
WeakCallbackType::kParameter);
70+
}
71+
72+
// This just has to be different from the Chromium ones:
73+
// https://source.chromium.org/chromium/chromium/src/+/main:gin/public/gin_embedders.h;l=18-23;drc=5a758a97032f0b656c3c36a3497560762495501a
74+
// Otherwise, when Node is loaded in an isolate which uses cppgc, cppgc will
75+
// misinterpret the data stored in the embedder fields and try to garbage
76+
// collect them.
77+
uint16_t kNodeEmbedderId = 0x90de;
78+
79+
void BaseObject::LazilyInitializedJSTemplateConstructor(
80+
const FunctionCallbackInfo<Value>& args) {
81+
DCHECK(args.IsConstructCall());
82+
CHECK_GE(args.This()->InternalFieldCount(), BaseObject::kInternalFieldCount);
83+
args.This()->SetAlignedPointerInInternalField(BaseObject::kEmbedderType,
84+
&kNodeEmbedderId);
85+
args.This()->SetAlignedPointerInInternalField(BaseObject::kSlot, nullptr);
86+
}
87+
88+
Local<FunctionTemplate> BaseObject::MakeLazilyInitializedJSTemplate(
89+
Environment* env) {
90+
Local<FunctionTemplate> t = NewFunctionTemplate(
91+
env->isolate(), LazilyInitializedJSTemplateConstructor);
92+
t->Inherit(BaseObject::GetConstructorTemplate(env));
93+
t->InstanceTemplate()->SetInternalFieldCount(BaseObject::kInternalFieldCount);
94+
return t;
95+
}
96+
97+
BaseObject::PointerData* BaseObject::pointer_data() {
98+
if (!has_pointer_data()) {
99+
PointerData* metadata = new PointerData();
100+
metadata->wants_weak_jsobj = persistent_handle_.IsWeak();
101+
metadata->self = this;
102+
pointer_data_ = metadata;
103+
}
104+
CHECK(has_pointer_data());
105+
return pointer_data_;
106+
}
107+
108+
void BaseObject::decrease_refcount() {
109+
CHECK(has_pointer_data());
110+
PointerData* metadata = pointer_data();
111+
CHECK_GT(metadata->strong_ptr_count, 0);
112+
unsigned int new_refcount = --metadata->strong_ptr_count;
113+
if (new_refcount == 0) {
114+
if (metadata->is_detached) {
115+
OnGCCollect();
116+
} else if (metadata->wants_weak_jsobj && !persistent_handle_.IsEmpty()) {
117+
MakeWeak();
118+
}
119+
}
120+
}
121+
122+
void BaseObject::increase_refcount() {
123+
unsigned int prev_refcount = pointer_data()->strong_ptr_count++;
124+
if (prev_refcount == 0 && !persistent_handle_.IsEmpty())
125+
persistent_handle_.ClearWeak();
126+
}
127+
128+
void BaseObject::DeleteMe(void* data) {
129+
BaseObject* self = static_cast<BaseObject*>(data);
130+
if (self->has_pointer_data() && self->pointer_data()->strong_ptr_count > 0) {
131+
return self->Detach();
132+
}
133+
delete self;
134+
}
135+
136+
bool BaseObject::IsDoneInitializing() const {
137+
return true;
138+
}
139+
140+
Local<Object> BaseObject::WrappedObject() const {
141+
return object();
142+
}
143+
144+
bool BaseObject::IsRootNode() const {
145+
return !persistent_handle_.IsWeak();
146+
}
147+
148+
Local<FunctionTemplate> BaseObject::GetConstructorTemplate(
149+
IsolateData* isolate_data) {
150+
Local<FunctionTemplate> tmpl = isolate_data->base_object_ctor_template();
151+
if (tmpl.IsEmpty()) {
152+
tmpl = NewFunctionTemplate(isolate_data->isolate(), nullptr);
153+
tmpl->SetClassName(
154+
FIXED_ONE_BYTE_STRING(isolate_data->isolate(), "BaseObject"));
155+
isolate_data->set_base_object_ctor_template(tmpl);
156+
}
157+
return tmpl;
158+
}
159+
160+
bool BaseObject::IsNotIndicativeOfMemoryLeakAtExit() const {
161+
return IsWeakOrDetached();
162+
}
163+
164+
} // namespace node
Collapse file

‎src/base_object.h‎

Copy file name to clipboardExpand all lines: src/base_object.h
+8-2Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ namespace node {
3232

3333
class Environment;
3434
class IsolateData;
35+
class Realm;
3536
template <typename T, bool kIsWeak>
3637
class BaseObjectPtrImpl;
3738

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

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

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

6569
inline Environment* env() const;
70+
inline Realm* realm() const;
6671

6772
// Get a BaseObject* pointer, or subclass pointer, for the JS object that
6873
// was also passed to the `BaseObject()` constructor initially.
@@ -91,6 +96,7 @@ class BaseObject : public MemoryRetainer {
9196
// Utility to create a FunctionTemplate with one internal field (used for
9297
// the `BaseObject*` pointer) and a constructor that initializes that field
9398
// to `nullptr`.
99+
// TODO(legendecas): Disentangle template with env.
94100
static v8::Local<v8::FunctionTemplate> MakeLazilyInitializedJSTemplate(
95101
Environment* env);
96102

@@ -213,7 +219,7 @@ class BaseObject : public MemoryRetainer {
213219
void decrease_refcount();
214220
void increase_refcount();
215221

216-
Environment* env_;
222+
Realm* realm_;
217223
PointerData* pointer_data_ = nullptr;
218224
};
219225

Collapse file

‎src/env-inl.h‎

Copy file name to clipboardExpand all lines: src/env-inl.h
+6-21Lines changed: 6 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -745,6 +745,12 @@ inline IsolateData* Environment::isolate_data() const {
745745
return isolate_data_;
746746
}
747747

748+
template <typename T>
749+
inline void Environment::ForEachRealm(T&& iterator) const {
750+
// TODO(legendecas): iterate over more realms bound to the environment.
751+
iterator(principal_realm());
752+
}
753+
748754
inline void Environment::ThrowError(const char* errmsg) {
749755
ThrowError(v8::Exception::Error, errmsg);
750756
}
@@ -789,27 +795,6 @@ void Environment::RemoveCleanupHook(CleanupQueue::Callback fn, void* arg) {
789795
cleanup_queue_.Remove(fn, arg);
790796
}
791797

792-
template <typename T>
793-
void Environment::ForEachBaseObject(T&& iterator) {
794-
cleanup_queue_.ForEachBaseObject(std::forward<T>(iterator));
795-
}
796-
797-
void Environment::modify_base_object_count(int64_t delta) {
798-
base_object_count_ += delta;
799-
}
800-
801-
int64_t Environment::base_object_count() const {
802-
return base_object_count_;
803-
}
804-
805-
inline void Environment::set_base_object_created_by_bootstrap(int64_t count) {
806-
base_object_created_by_bootstrap_ = base_object_count_;
807-
}
808-
809-
int64_t Environment::base_object_created_after_bootstrap() const {
810-
return base_object_count_ - base_object_created_by_bootstrap_;
811-
}
812-
813798
void Environment::set_main_utf16(std::unique_ptr<v8::String::Value> str) {
814799
CHECK(!main_utf16_);
815800
main_utf16_ = std::move(str);

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.