FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

src: introduce custom smart pointers for `BaseObject`s · nodejs/node@a2dbadc · GitHub

/ node Public

Commit a2dbadc

Browse files
addaleax
authored and
MylesBorins
committed
src: introduce custom smart pointers for BaseObjects
Referring to `BaseObject` instances using standard C++ smart pointers can interfere with BaseObject’s own cleanup mechanisms (explicit delete, delete-on-GC and delete-on-cleanup). Introducing custom smart pointers allows referring to `BaseObject`s safely while keeping those mechanisms intact. Refs: nodejs/quic#141 Refs: nodejs/quic#149 Reviewed-By: James M Snell <jasnell@gmail.com> PR-URL: #30374 Refs: nodejs/quic#165 Reviewed-By: David Carlier <devnexen@gmail.com>
1 parent 1a92c88 commit a2dbadc

13 files changed

Lines changed: 536 additions & 33 deletions

‎node.gyp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1100,6 +1100,7 @@
11001100
'test/cctest/node_test_fixture.h',
11011101
'test/cctest/test_aliased_buffer.cc',
11021102
'test/cctest/test_base64.cc',
1103+
'test/cctest/test_base_object_ptr.cc',
11031104
'test/cctest/test_node_postmortem_metadata.cc',
11041105
'test/cctest/test_environment.cc',
11051106
'test/cctest/test_linked_binding.cc',

‎src/base_object-inl.h‎

Lines changed: 206 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -32,24 +32,33 @@
3232
namespace node {
3333

3434
BaseObject::BaseObject(Environment* env, v8::Local<v8::Object> object)
35-
: persistent_handle_(env->isolate(), object),
36-
env_(env) {
35+
: persistent_handle_(env->isolate(), object), env_(env) {
3736
CHECK_EQ(false, object.IsEmpty());
3837
CHECK_GT(object->InternalFieldCount(), 0);
3938
object->SetAlignedPointerInInternalField(0, static_cast<void*>(this));
40-
env_->AddCleanupHook(DeleteMe, static_cast<void*>(this));
39+
env->AddCleanupHook(DeleteMe, static_cast<void*>(this));
40+
env->modify_base_object_count(1);
4141
}
4242

4343
BaseObject::~BaseObject() {
44-
RemoveCleanupHook();
44+
env()->modify_base_object_count(-1);
45+
env()->RemoveCleanupHook(DeleteMe, static_cast<void*>(this));
46+
47+
if (UNLIKELY(has_pointer_data())) {
48+
PointerData* metadata = pointer_data();
49+
CHECK_EQ(metadata->strong_ptr_count, 0);
50+
metadata->self = nullptr;
51+
if (metadata->weak_ptr_count == 0)
52+
delete metadata;
53+
}
4554

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

5160
{
52-
v8::HandleScope handle_scope(env_->isolate());
61+
v8::HandleScope handle_scope(env()->isolate());
5362
object()->SetAlignedPointerInInternalField(0, nullptr);
5463
}
5564
}
@@ -58,20 +67,25 @@ void BaseObject::RemoveCleanupHook() {
5867
env_->RemoveCleanupHook(DeleteMe, static_cast<void*>(this));
5968
}
6069

70+
void BaseObject::Detach() {
71+
CHECK_GT(pointer_data()->strong_ptr_count, 0);
72+
pointer_data()->is_detached = true;
73+
}
74+
6175
v8::Global<v8::Object>& BaseObject::persistent() {
6276
return persistent_handle_;
6377
}
6478

6579

6680
v8::Local<v8::Object> BaseObject::object() const {
67-
return PersistentToLocal::Default(env_->isolate(), persistent_handle_);
81+
return PersistentToLocal::Default(env()->isolate(), persistent_handle_);
6882
}
6983

7084
v8::Local<v8::Object> BaseObject::object(v8::Isolate* isolate) const {
7185
v8::Local<v8::Object> handle = object();
7286

7387
DCHECK_EQ(handle->CreationContext()->GetIsolate(), isolate);
74-
DCHECK_EQ(env_->isolate(), isolate);
88+
DCHECK_EQ(env()->isolate(), isolate);
7589

7690
return handle;
7791
}
@@ -80,7 +94,6 @@ Environment* BaseObject::env() const {
8094
return env_;
8195
}
8296

83-
8497
BaseObject* BaseObject::FromJSObject(v8::Local<v8::Object> obj) {
8598
CHECK_GT(obj->InternalFieldCount(), 0);
8699
return static_cast<BaseObject*>(obj->GetAlignedPointerFromInternalField(0));
@@ -94,20 +107,34 @@ T* BaseObject::FromJSObject(v8::Local<v8::Object> object) {
94107

95108

96109
void BaseObject::MakeWeak() {
110+
if (has_pointer_data()) {
111+
pointer_data()->wants_weak_jsobj = true;
112+
if (pointer_data()->strong_ptr_count > 0) return;
113+
}
114+
97115
persistent_handle_.SetWeak(
98116
this,
99117
[](const v8::WeakCallbackInfo<BaseObject>& data) {
100-
std::unique_ptr<BaseObject> obj(data.GetParameter());
118+
BaseObject* obj = data.GetParameter();
101119
// Clear the persistent handle so that ~BaseObject() doesn't attempt
102120
// to mess with internal fields, since the JS object may have
103121
// transitioned into an invalid state.
104122
// Refs: https://github.com/nodejs/node/issues/18897
105123
obj->persistent_handle_.Reset();
124+
CHECK_IMPLIES(obj->has_pointer_data(),
125+
obj->pointer_data()->strong_ptr_count == 0);
126+
obj->OnGCCollect();
106127
}, v8::WeakCallbackType::kParameter);
107128
}
108129

130+
void BaseObject::OnGCCollect() {
131+
delete this;
132+
}
109133

110134
void BaseObject::ClearWeak() {
135+
if (has_pointer_data())
136+
pointer_data()->wants_weak_jsobj = false;
137+
111138
persistent_handle_.ClearWeak();
112139
}
113140

@@ -141,6 +168,176 @@ void BaseObject::InternalFieldSet(v8::Local<v8::String> property,
141168
info.This()->SetInternalField(Field, value);
142169
}
143170

171+
bool BaseObject::has_pointer_data() const {
172+
return pointer_data_ != nullptr;
173+
}
174+
175+
BaseObject::PointerData* BaseObject::pointer_data() {
176+
if (!has_pointer_data()) {
177+
PointerData* metadata = new PointerData();
178+
metadata->wants_weak_jsobj = persistent_handle_.IsWeak();
179+
metadata->self = this;
180+
pointer_data_ = metadata;
181+
}
182+
CHECK(has_pointer_data());
183+
return pointer_data_;
184+
}
185+
186+
void BaseObject::decrease_refcount() {
187+
CHECK(has_pointer_data());
188+
PointerData* metadata = pointer_data();
189+
CHECK_GT(metadata->strong_ptr_count, 0);
190+
unsigned int new_refcount = --metadata->strong_ptr_count;
191+
if (new_refcount == 0) {
192+
if (metadata->is_detached) {
193+
delete this;
194+
} else if (metadata->wants_weak_jsobj && !persistent_handle_.IsEmpty()) {
195+
MakeWeak();
196+
}
197+
}
198+
}
199+
200+
void BaseObject::increase_refcount() {
201+
unsigned int prev_refcount = pointer_data()->strong_ptr_count++;
202+
if (prev_refcount == 0 && !persistent_handle_.IsEmpty())
203+
persistent_handle_.ClearWeak();
204+
}
205+
206+
template <typename T, bool kIsWeak>
207+
BaseObject::PointerData*
208+
BaseObjectPtrImpl<T, kIsWeak>::pointer_data() const {
209+
if (kIsWeak) {
210+
return data_.pointer_data;
211+
} else {
212+
if (get_base_object() == nullptr) return nullptr;
213+
return get_base_object()->pointer_data();
214+
}
215+
}
216+
217+
template <typename T, bool kIsWeak>
218+
BaseObject* BaseObjectPtrImpl<T, kIsWeak>::get_base_object() const {
219+
if (kIsWeak) {
220+
if (pointer_data() == nullptr) return nullptr;
221+
return pointer_data()->self;
222+
} else {
223+
return data_.target;
224+
}
225+
}
226+
227+
template <typename T, bool kIsWeak>
228+
BaseObjectPtrImpl<T, kIsWeak>::~BaseObjectPtrImpl() {
229+
if (get() == nullptr) return;
230+
if (kIsWeak) {
231+
if (--pointer_data()->weak_ptr_count == 0 &&
232+
pointer_data()->self == nullptr) {
233+
delete pointer_data();
234+
}
235+
} else {
236+
get()->decrease_refcount();
237+
}
238+
}
239+
240+
template <typename T, bool kIsWeak>
241+
BaseObjectPtrImpl<T, kIsWeak>::BaseObjectPtrImpl() {
242+
data_.target = nullptr;
243+
}
244+
245+
template <typename T, bool kIsWeak>
246+
BaseObjectPtrImpl<T, kIsWeak>::BaseObjectPtrImpl(T* target)
247+
: BaseObjectPtrImpl() {
248+
if (target == nullptr) return;
249+
if (kIsWeak) {
250+
data_.pointer_data = target->pointer_data();
251+
CHECK_NOT_NULL(pointer_data());
252+
pointer_data()->weak_ptr_count++;
253+
} else {
254+
data_.target = target;
255+
CHECK_NOT_NULL(pointer_data());
256+
get()->increase_refcount();
257+
}
258+
}
259+
260+
template <typename T, bool kIsWeak>
261+
template <typename U, bool kW>
262+
BaseObjectPtrImpl<T, kIsWeak>::BaseObjectPtrImpl(
263+
const BaseObjectPtrImpl<U, kW>& other)
264+
: BaseObjectPtrImpl(other.get()) {}
265+
266+
template <typename T, bool kIsWeak>
267+
BaseObjectPtrImpl<T, kIsWeak>::BaseObjectPtrImpl(const BaseObjectPtrImpl& other)
268+
: BaseObjectPtrImpl(other.get()) {}
269+
270+
template <typename T, bool kIsWeak>
271+
template <typename U, bool kW>
272+
BaseObjectPtrImpl<T, kIsWeak>& BaseObjectPtrImpl<T, kIsWeak>::operator=(
273+
const BaseObjectPtrImpl<U, kW>& other) {
274+
if (other.get() == get()) return *this;
275+
this->~BaseObjectPtrImpl();
276+
return *new (this) BaseObjectPtrImpl(other);
277+
}
278+
279+
template <typename T, bool kIsWeak>
280+
BaseObjectPtrImpl<T, kIsWeak>& BaseObjectPtrImpl<T, kIsWeak>::operator=(
281+
const BaseObjectPtrImpl& other) {
282+
if (other.get() == get()) return *this;
283+
this->~BaseObjectPtrImpl();
284+
return *new (this) BaseObjectPtrImpl(other);
285+
}
286+
287+
template <typename T, bool kIsWeak>
288+
BaseObjectPtrImpl<T, kIsWeak>::BaseObjectPtrImpl(BaseObjectPtrImpl&& other)
289+
: data_(other.data_) {
290+
if (kIsWeak)
291+
other.data_.target = nullptr;
292+
else
293+
other.data_.pointer_data = nullptr;
294+
}
295+
296+
template <typename T, bool kIsWeak>
297+
BaseObjectPtrImpl<T, kIsWeak>& BaseObjectPtrImpl<T, kIsWeak>::operator=(
298+
BaseObjectPtrImpl&& other) {
299+
if (&other == this) return *this;
300+
this->~BaseObjectPtrImpl();
301+
return *new (this) BaseObjectPtrImpl(std::move(other));
302+
}
303+
304+
template <typename T, bool kIsWeak>
305+
void BaseObjectPtrImpl<T, kIsWeak>::reset(T* ptr) {
306+
*this = BaseObjectPtrImpl(ptr);
307+
}
308+
309+
template <typename T, bool kIsWeak>
310+
T* BaseObjectPtrImpl<T, kIsWeak>::get() const {
311+
return static_cast<T*>(get_base_object());
312+
}
313+
314+
template <typename T, bool kIsWeak>
315+
T& BaseObjectPtrImpl<T, kIsWeak>::operator*() const {
316+
return *get();
317+
}
318+
319+
template <typename T, bool kIsWeak>
320+
T* BaseObjectPtrImpl<T, kIsWeak>::operator->() const {
321+
return get();
322+
}
323+
324+
template <typename T, bool kIsWeak>
325+
BaseObjectPtrImpl<T, kIsWeak>::operator bool() const {
326+
return get() != nullptr;
327+
}
328+
329+
template <typename T, typename... Args>
330+
BaseObjectPtr<T> MakeBaseObject(Args&&... args) {
331+
return BaseObjectPtr<T>(new T(std::forward<Args>(args)...));
332+
}
333+
334+
template <typename T, typename... Args>
335+
BaseObjectPtr<T> MakeDetachedBaseObject(Args&&... args) {
336+
BaseObjectPtr<T> target = MakeBaseObject<T>(std::forward<Args>(args)...);
337+
target->Detach();
338+
return target;
339+
}
340+
144341
} // namespace node
145342

146343
#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL