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 <[email protected]> Backport-PR-URL: #32301 PR-URL: #30374 Refs: nodejs/quic#165 Reviewed-By: David Carlier <[email protected]>

Author: addaleax

node.gyp

@@ -1117,6 +1117,7 @@
 'test/cctest/node_test_fixture.h',
 'test/cctest/test_aliased_buffer.cc',
 'test/cctest/test_base64.cc',
+'test/cctest/test_base_object_ptr.cc',
 'test/cctest/test_node_postmortem_metadata.cc',
 'test/cctest/test_environment.cc',
 'test/cctest/test_linked_binding.cc',

src/base_object-inl.h

@@ -40,24 +40,33 @@
 namespacenode{
 
 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.
 return;
  }
 
 {
- v8::HandleScope handle_scope(env_->isolate());
+ v8::HandleScope handle_scope(env()->isolate());
 object()->SetAlignedPointerInInternalField(0, nullptr);
  }
 }
@@ -66,20 +75,25 @@ void BaseObject::RemoveCleanupHook(){
  env_->RemoveCleanupHook(DeleteMe, static_cast<void*>(this));
 }
 
+voidBaseObject::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{
-returnPersistentToLocal::Default(env_->isolate(), persistent_handle_);
+returnPersistentToLocal::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;
 }
@@ -88,7 +102,6 @@ Environment* BaseObject::env() const{
 return env_;
 }
 
-
 BaseObject* BaseObject::FromJSObject(v8::Local<v8::Object> obj){
 CHECK_GT(obj->InternalFieldCount(), 0);
 returnstatic_cast<BaseObject*>(obj->GetAlignedPointerFromInternalField(0));
@@ -102,20 +115,34 @@ T* BaseObject::FromJSObject(v8::Local<v8::Object> object){
 
 
 voidBaseObject::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);
 }
 
+voidBaseObject::OnGCCollect(){
+deletethis;
+}
 
 voidBaseObject::ClearWeak(){
+if (has_pointer_data())
+pointer_data()->wants_weak_jsobj = false;
+
  persistent_handle_.ClearWeak();
 }
 
@@ -149,6 +176,176 @@ void BaseObject::InternalFieldSet(v8::Local<v8::String> property,
  info.This()->SetInternalField(Field, value);
 }
 
+boolBaseObject::has_pointer_data() const{
+return pointer_data_ != nullptr;
+}
+
+BaseObject::PointerData* BaseObject::pointer_data(){
+if (!has_pointer_data()){
+ PointerData* metadata = newPointerData();
+ metadata->wants_weak_jsobj = persistent_handle_.IsWeak();
+ metadata->self = this;
+ pointer_data_ = metadata;
+ }
+CHECK(has_pointer_data());
+return pointer_data_;
+}
+
+voidBaseObject::decrease_refcount(){
+CHECK(has_pointer_data());
+ PointerData* metadata = pointer_data();
+CHECK_GT(metadata->strong_ptr_count, 0);
+unsignedint new_refcount = --metadata->strong_ptr_count;
+if (new_refcount == 0){
+if (metadata->is_detached){
+deletethis;
+ } elseif (metadata->wants_weak_jsobj && !persistent_handle_.IsEmpty()){
+MakeWeak();
+ }
+ }
+}
+
+voidBaseObject::increase_refcount(){
+unsignedint prev_refcount = pointer_data()->strong_ptr_count++;
+if (prev_refcount == 0 && !persistent_handle_.IsEmpty())
+ persistent_handle_.ClearWeak();
+}
+
+template <typename T, boolkIsWeak>
+BaseObject::PointerData*
+BaseObjectPtrImpl<T, kIsWeak>::pointer_data() const{
+if (kIsWeak){
+return data_.pointer_data;
+ } else{
+if (get_base_object() == nullptr) returnnullptr;
+returnget_base_object()->pointer_data();
+ }
+}
+
+template <typename T, boolkIsWeak>
+BaseObject* BaseObjectPtrImpl<T, kIsWeak>::get_base_object() const{
+if (kIsWeak){
+if (pointer_data() == nullptr) returnnullptr;
+returnpointer_data()->self;
+ } else{
+return data_.target;
+ }
+}
+
+template <typename T, boolkIsWeak>
+BaseObjectPtrImpl<T, kIsWeak>::~BaseObjectPtrImpl(){
+if (get() == nullptr) return;
+if (kIsWeak){
+if (--pointer_data()->weak_ptr_count == 0 &&
+pointer_data()->self == nullptr){
+deletepointer_data();
+ }
+ } else{
+get()->decrease_refcount();
+ }
+}
+
+template <typename T, boolkIsWeak>
+BaseObjectPtrImpl<T, kIsWeak>::BaseObjectPtrImpl(){
+ data_.target = nullptr;
+}
+
+template <typename T, boolkIsWeak>
+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, boolkIsWeak>
+template <typename U, boolkW>
+BaseObjectPtrImpl<T, kIsWeak>::BaseObjectPtrImpl(
+const BaseObjectPtrImpl<U, kW>& other)
+ : BaseObjectPtrImpl(other.get()){}
+
+template <typename T, boolkIsWeak>
+BaseObjectPtrImpl<T, kIsWeak>::BaseObjectPtrImpl(const BaseObjectPtrImpl& other)
+ : BaseObjectPtrImpl(other.get()){}
+
+template <typename T, boolkIsWeak>
+template <typename U, boolkW>
+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, boolkIsWeak>
+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, boolkIsWeak>
+BaseObjectPtrImpl<T, kIsWeak>::BaseObjectPtrImpl(BaseObjectPtrImpl&& other)
+ : data_(other.data_){
+if (kIsWeak)
+ other.data_.target = nullptr;
+else
+ other.data_.pointer_data = nullptr;
+}
+
+template <typename T, boolkIsWeak>
+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, boolkIsWeak>
+void BaseObjectPtrImpl<T, kIsWeak>::reset(T* ptr){
+ *this = BaseObjectPtrImpl(ptr);
+}
+
+template <typename T, boolkIsWeak>
+T* BaseObjectPtrImpl<T, kIsWeak>::get() const{
+returnstatic_cast<T*>(get_base_object());
+}
+
+template <typename T, boolkIsWeak>
+T& BaseObjectPtrImpl<T, kIsWeak>::operator*() const{
+return *get();
+}
+
+template <typename T, boolkIsWeak>
+T* BaseObjectPtrImpl<T, kIsWeak>::operator->() const{
+returnget();
+}
+
+template <typename T, boolkIsWeak>
+BaseObjectPtrImpl<T, kIsWeak>::operatorbool() const{
+returnget() != nullptr;
+}
+
+template <typename T, typename... Args>
+BaseObjectPtr<T> MakeBaseObject(Args&&... args){
+return BaseObjectPtr<T>(newT(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