diff options
author | Anna Henningsen <anna@addaleax.net> | 2019-09-29 02:29:07 +0300 |
---|---|---|
committer | Anna Henningsen <anna@addaleax.net> | 2019-11-19 15:47:29 +0300 |
commit | 7e6104f1bb63a07885b08f277c3b708caa6b8a23 (patch) | |
tree | 32d76e6f055d3bfacd849633db3a8af3868a328c /src/base_object-inl.h | |
parent | efce655c0f1671d0e86b5c89092ac93db983ef94 (diff) |
src: introduce custom smart pointers for `BaseObject`s
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: https://github.com/nodejs/quic/pull/141
Refs: https://github.com/nodejs/quic/pull/149
Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/30374
Refs: https://github.com/nodejs/quic/pull/165
Reviewed-By: David Carlier <devnexen@gmail.com>
Diffstat (limited to 'src/base_object-inl.h')
-rw-r--r-- | src/base_object-inl.h | 215 |
1 files changed, 206 insertions, 9 deletions
diff --git a/src/base_object-inl.h b/src/base_object-inl.h index af69084f4a5..4fc9210b39b 100644 --- a/src/base_object-inl.h +++ b/src/base_object-inl.h @@ -32,16 +32,25 @@ namespace node { BaseObject::BaseObject(Environment* env, v8::Local<v8::Object> object) - : persistent_handle_(env->isolate(), object), - env_(env) { + : persistent_handle_(env->isolate(), object), env_(env) { CHECK_EQ(false, object.IsEmpty()); CHECK_GT(object->InternalFieldCount(), 0); object->SetAlignedPointerInInternalField(0, static_cast<void*>(this)); - env_->AddCleanupHook(DeleteMe, static_cast<void*>(this)); + env->AddCleanupHook(DeleteMe, static_cast<void*>(this)); + env->modify_base_object_count(1); } BaseObject::~BaseObject() { - RemoveCleanupHook(); + env()->modify_base_object_count(-1); + env()->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. @@ -49,7 +58,7 @@ BaseObject::~BaseObject() { } { - v8::HandleScope handle_scope(env_->isolate()); + v8::HandleScope handle_scope(env()->isolate()); object()->SetAlignedPointerInInternalField(0, nullptr); } } @@ -58,20 +67,25 @@ void BaseObject::RemoveCleanupHook() { env_->RemoveCleanupHook(DeleteMe, static_cast<void*>(this)); } +void BaseObject::Detach() { + CHECK_GT(pointer_data()->strong_ptr_count, 0); + pointer_data()->is_detached = true; +} + v8::Global<v8::Object>& BaseObject::persistent() { return persistent_handle_; } v8::Local<v8::Object> BaseObject::object() const { - return PersistentToLocal::Default(env_->isolate(), persistent_handle_); + return PersistentToLocal::Default(env()->isolate(), persistent_handle_); } v8::Local<v8::Object> BaseObject::object(v8::Isolate* isolate) const { v8::Local<v8::Object> handle = object(); DCHECK_EQ(handle->CreationContext()->GetIsolate(), isolate); - DCHECK_EQ(env_->isolate(), isolate); + DCHECK_EQ(env()->isolate(), isolate); return handle; } @@ -80,7 +94,6 @@ Environment* BaseObject::env() const { return env_; } - BaseObject* BaseObject::FromJSObject(v8::Local<v8::Object> obj) { CHECK_GT(obj->InternalFieldCount(), 0); return static_cast<BaseObject*>(obj->GetAlignedPointerFromInternalField(0)); @@ -94,20 +107,34 @@ T* BaseObject::FromJSObject(v8::Local<v8::Object> object) { 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 v8::WeakCallbackInfo<BaseObject>& data) { - std::unique_ptr<BaseObject> obj(data.GetParameter()); + 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(); }, v8::WeakCallbackType::kParameter); } +void BaseObject::OnGCCollect() { + delete this; +} void BaseObject::ClearWeak() { + if (has_pointer_data()) + pointer_data()->wants_weak_jsobj = false; + persistent_handle_.ClearWeak(); } @@ -141,6 +168,176 @@ void BaseObject::InternalFieldSet(v8::Local<v8::String> property, info.This()->SetInternalField(Field, value); } +bool BaseObject::has_pointer_data() const { + return pointer_data_ != nullptr; +} + +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) { + delete this; + } 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(); +} + +template <typename T, bool kIsWeak> +BaseObject::PointerData* +BaseObjectPtrImpl<T, kIsWeak>::pointer_data() const { + if (kIsWeak) { + return data_.pointer_data; + } else { + if (get_base_object() == nullptr) return nullptr; + return get_base_object()->pointer_data(); + } +} + +template <typename T, bool kIsWeak> +BaseObject* BaseObjectPtrImpl<T, kIsWeak>::get_base_object() const { + if (kIsWeak) { + if (pointer_data() == nullptr) return nullptr; + return pointer_data()->self; + } else { + return data_.target; + } +} + +template <typename T, bool kIsWeak> +BaseObjectPtrImpl<T, kIsWeak>::~BaseObjectPtrImpl() { + if (get() == nullptr) return; + if (kIsWeak) { + if (--pointer_data()->weak_ptr_count == 0 && + pointer_data()->self == nullptr) { + delete pointer_data(); + } + } else { + get()->decrease_refcount(); + } +} + +template <typename T, bool kIsWeak> +BaseObjectPtrImpl<T, kIsWeak>::BaseObjectPtrImpl() { + data_.target = nullptr; +} + +template <typename T, bool kIsWeak> +BaseObjectPtrImpl<T, kIsWeak>::BaseObjectPtrImpl(T* target) + : BaseObjectPtrImpl() { + if (target == nullptr) return; + if (kIsWeak) { + data_.pointer_data = target->pointer_data(); + CHECK_NOT_NULL(pointer_data()); + pointer_data()->weak_ptr_count++; + } else { + data_.target = target; + CHECK_NOT_NULL(pointer_data()); + get()->increase_refcount(); + } +} + +template <typename T, bool kIsWeak> +template <typename U, bool kW> +BaseObjectPtrImpl<T, kIsWeak>::BaseObjectPtrImpl( + const BaseObjectPtrImpl<U, kW>& other) + : BaseObjectPtrImpl(other.get()) {} + +template <typename T, bool kIsWeak> +BaseObjectPtrImpl<T, kIsWeak>::BaseObjectPtrImpl(const BaseObjectPtrImpl& other) + : BaseObjectPtrImpl(other.get()) {} + +template <typename T, bool kIsWeak> +template <typename U, bool kW> +BaseObjectPtrImpl<T, kIsWeak>& BaseObjectPtrImpl<T, kIsWeak>::operator=( + const BaseObjectPtrImpl<U, kW>& other) { + if (other.get() == get()) return *this; + this->~BaseObjectPtrImpl(); + return *new (this) BaseObjectPtrImpl(other); +} + +template <typename T, bool kIsWeak> +BaseObjectPtrImpl<T, kIsWeak>& BaseObjectPtrImpl<T, kIsWeak>::operator=( + const BaseObjectPtrImpl& other) { + if (other.get() == get()) return *this; + this->~BaseObjectPtrImpl(); + return *new (this) BaseObjectPtrImpl(other); +} + +template <typename T, bool kIsWeak> +BaseObjectPtrImpl<T, kIsWeak>::BaseObjectPtrImpl(BaseObjectPtrImpl&& other) + : data_(other.data_) { + if (kIsWeak) + other.data_.target = nullptr; + else + other.data_.pointer_data = nullptr; +} + +template <typename T, bool kIsWeak> +BaseObjectPtrImpl<T, kIsWeak>& BaseObjectPtrImpl<T, kIsWeak>::operator=( + BaseObjectPtrImpl&& other) { + if (&other == this) return *this; + this->~BaseObjectPtrImpl(); + return *new (this) BaseObjectPtrImpl(std::move(other)); +} + +template <typename T, bool kIsWeak> +void BaseObjectPtrImpl<T, kIsWeak>::reset(T* ptr) { + *this = BaseObjectPtrImpl(ptr); +} + +template <typename T, bool kIsWeak> +T* BaseObjectPtrImpl<T, kIsWeak>::get() const { + return static_cast<T*>(get_base_object()); +} + +template <typename T, bool kIsWeak> +T& BaseObjectPtrImpl<T, kIsWeak>::operator*() const { + return *get(); +} + +template <typename T, bool kIsWeak> +T* BaseObjectPtrImpl<T, kIsWeak>::operator->() const { + return get(); +} + +template <typename T, bool kIsWeak> +BaseObjectPtrImpl<T, kIsWeak>::operator bool() const { + return get() != nullptr; +} + +template <typename T, typename... Args> +BaseObjectPtr<T> MakeBaseObject(Args&&... args) { + return BaseObjectPtr<T>(new T(std::forward<Args>(args)...)); +} + +template <typename T, typename... Args> +BaseObjectPtr<T> MakeDetachedBaseObject(Args&&... args) { + BaseObjectPtr<T> target = MakeBaseObject<T>(std::forward<Args>(args)...); + target->Detach(); + return target; +} + } // namespace node #endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS |