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>

3 files changed, 181 insertions, 6 deletions

diff --git a/test/cctest/node_test_fixture.h b/test/cctest/node_test_fixture.h
index f6b80c860c..ac0701d094 100644
--- a/test/cctest/node_test_fixture.h
+++ b/test/cctest/node_test_fixture.h
@@ -105,9 +105,9 @@ class NodeTestFixture : public ::testing::Test {
   }
 
   void TearDown() override {
+    platform->DrainTasks(isolate_);
     isolate_->Exit();
     isolate_->Dispose();
-    platform->DrainTasks(isolate_);
     platform->UnregisterIsolate(isolate_);
     isolate_ = nullptr;
   }
diff --git a/test/cctest/test_base_object_ptr.cc b/test/cctest/test_base_object_ptr.cc
new file mode 100644
index 0000000000..18e27edba8
--- /dev/null
+++ b/test/cctest/test_base_object_ptr.cc
@@ -0,0 +1,176 @@
+#include "gtest/gtest.h"
+#include "node.h"
+#include "base_object-inl.h"
+#include "node_test_fixture.h"
+
+using node::BaseObject;
+using node::BaseObjectPtr;
+using node::BaseObjectWeakPtr;
+using node::Environment;
+using node::MakeBaseObject;
+using node::MakeDetachedBaseObject;
+using v8::HandleScope;
+using v8::Isolate;
+using v8::Local;
+using v8::Object;
+
+class BaseObjectPtrTest : public EnvironmentTestFixture {};
+
+class DummyBaseObject : public BaseObject {
+ public:
+  DummyBaseObject(Environment* env, Local<Object> obj) : BaseObject(env, obj) {}
+
+  static Local<Object> MakeJSObject(Environment* env) {
+    return BaseObject::MakeLazilyInitializedJSTemplate(env)
+        ->GetFunction(env->context()).ToLocalChecked()
+        ->NewInstance(env->context()).ToLocalChecked();
+  }
+
+  static BaseObjectPtr<DummyBaseObject> NewDetached(Environment* env) {
+    Local<Object> obj = MakeJSObject(env);
+    return MakeDetachedBaseObject<DummyBaseObject>(env, obj);
+  }
+
+  static BaseObjectPtr<DummyBaseObject> New(Environment* env) {
+    Local<Object> obj = MakeJSObject(env);
+    return MakeBaseObject<DummyBaseObject>(env, obj);
+  }
+
+  SET_NO_MEMORY_INFO()
+  SET_MEMORY_INFO_NAME(DummyBaseObject)
+  SET_SELF_SIZE(DummyBaseObject)
+};
+
+TEST_F(BaseObjectPtrTest, ScopedDetached) {
+  const HandleScope handle_scope(isolate_);
+  const Argv argv;
+  Env env_{handle_scope, argv};
+  Environment* env = *env_;
+
+  EXPECT_EQ(env->base_object_count(), 0);
+  {
+    BaseObjectPtr<DummyBaseObject> ptr = DummyBaseObject::NewDetached(env);
+    EXPECT_EQ(env->base_object_count(), 1);
+  }
+  EXPECT_EQ(env->base_object_count(), 0);
+}
+
+TEST_F(BaseObjectPtrTest, ScopedDetachedWithWeak) {
+  const HandleScope handle_scope(isolate_);
+  const Argv argv;
+  Env env_{handle_scope, argv};
+  Environment* env = *env_;
+
+  BaseObjectWeakPtr<DummyBaseObject> weak_ptr;
+
+  EXPECT_EQ(env->base_object_count(), 0);
+  {
+    BaseObjectPtr<DummyBaseObject> ptr = DummyBaseObject::NewDetached(env);
+    weak_ptr = ptr;
+    EXPECT_EQ(env->base_object_count(), 1);
+  }
+  EXPECT_EQ(weak_ptr.get(), nullptr);
+  EXPECT_EQ(env->base_object_count(), 0);
+}
+
+TEST_F(BaseObjectPtrTest, Undetached) {
+  const HandleScope handle_scope(isolate_);
+  const Argv argv;
+  Env env_{handle_scope, argv};
+  Environment* env = *env_;
+
+  node::AddEnvironmentCleanupHook(isolate_, [](void* arg) {
+    EXPECT_EQ(static_cast<Environment*>(arg)->base_object_count(), 0);
+  }, env);
+
+  BaseObjectPtr<DummyBaseObject> ptr = DummyBaseObject::New(env);
+  EXPECT_EQ(env->base_object_count(), 1);
+}
+
+TEST_F(BaseObjectPtrTest, GCWeak) {
+  const HandleScope handle_scope(isolate_);
+  const Argv argv;
+  Env env_{handle_scope, argv};
+  Environment* env = *env_;
+
+  BaseObjectWeakPtr<DummyBaseObject> weak_ptr;
+
+  {
+    const HandleScope handle_scope(isolate_);
+    BaseObjectPtr<DummyBaseObject> ptr = DummyBaseObject::New(env);
+    weak_ptr = ptr;
+    ptr->MakeWeak();
+
+    EXPECT_EQ(env->base_object_count(), 1);
+    EXPECT_EQ(weak_ptr.get(), ptr.get());
+    EXPECT_EQ(weak_ptr->persistent().IsWeak(), false);
+
+    ptr.reset();
+  }
+
+  EXPECT_EQ(env->base_object_count(), 1);
+  EXPECT_NE(weak_ptr.get(), nullptr);
+  EXPECT_EQ(weak_ptr->persistent().IsWeak(), true);
+
+  v8::V8::SetFlagsFromString("--expose-gc");
+  isolate_->RequestGarbageCollectionForTesting(Isolate::kFullGarbageCollection);
+
+  EXPECT_EQ(env->base_object_count(), 0);
+  EXPECT_EQ(weak_ptr.get(), nullptr);
+}
+
+TEST_F(BaseObjectPtrTest, Moveable) {
+  const HandleScope handle_scope(isolate_);
+  const Argv argv;
+  Env env_{handle_scope, argv};
+  Environment* env = *env_;
+
+  BaseObjectPtr<DummyBaseObject> ptr = DummyBaseObject::NewDetached(env);
+  EXPECT_EQ(env->base_object_count(), 1);
+  BaseObjectWeakPtr<DummyBaseObject> weak_ptr { ptr };
+  EXPECT_EQ(weak_ptr.get(), ptr.get());
+
+  BaseObjectPtr<DummyBaseObject> ptr2 = std::move(ptr);
+  EXPECT_EQ(weak_ptr.get(), ptr2.get());
+  EXPECT_EQ(ptr.get(), nullptr);
+
+  BaseObjectWeakPtr<DummyBaseObject> weak_ptr2 = std::move(weak_ptr);
+  EXPECT_EQ(weak_ptr2.get(), ptr2.get());
+  EXPECT_EQ(weak_ptr.get(), nullptr);
+  EXPECT_EQ(env->base_object_count(), 1);
+
+  ptr2.reset();
+
+  EXPECT_EQ(weak_ptr2.get(), nullptr);
+  EXPECT_EQ(env->base_object_count(), 0);
+}
+
+TEST_F(BaseObjectPtrTest, NestedClasses) {
+  class ObjectWithPtr : public BaseObject {
+   public:
+    ObjectWithPtr(Environment* env, Local<Object> obj) : BaseObject(env, obj) {}
+
+    BaseObjectPtr<BaseObject> ptr1;
+    BaseObjectPtr<BaseObject> ptr2;
+
+    SET_NO_MEMORY_INFO()
+    SET_MEMORY_INFO_NAME(ObjectWithPtr)
+    SET_SELF_SIZE(ObjectWithPtr)
+  };
+
+  const HandleScope handle_scope(isolate_);
+  const Argv argv;
+  Env env_{handle_scope, argv};
+  Environment* env = *env_;
+
+  node::AddEnvironmentCleanupHook(isolate_, [](void* arg) {
+    EXPECT_EQ(static_cast<Environment*>(arg)->base_object_count(), 0);
+  }, env);
+
+  ObjectWithPtr* obj =
+      new ObjectWithPtr(env, DummyBaseObject::MakeJSObject(env));
+  obj->ptr1 = DummyBaseObject::NewDetached(env);
+  obj->ptr2 = DummyBaseObject::New(env);
+
+  EXPECT_EQ(env->base_object_count(), 3);
+}
diff --git a/test/cctest/test_node_postmortem_metadata.cc b/test/cctest/test_node_postmortem_metadata.cc
index f33d40eb5c..3fb67ecbca 100644
--- a/test/cctest/test_node_postmortem_metadata.cc
+++ b/test/cctest/test_node_postmortem_metadata.cc
@@ -93,14 +93,13 @@ TEST_F(DebugSymbolsTest, BaseObjectPersistentHandle) {
 
   v8::Local<v8::Object> object =
       obj_templ->NewInstance(env.context()).ToLocalChecked();
-  DummyBaseObject obj(*env, object);
+  node::BaseObjectPtr<DummyBaseObject> obj =
+      node::MakeDetachedBaseObject<DummyBaseObject>(*env, object);
 
-  auto expected = reinterpret_cast<uintptr_t>(&obj.persistent());
-  auto calculated = reinterpret_cast<uintptr_t>(&obj) +
+  auto expected = reinterpret_cast<uintptr_t>(&obj->persistent());
+  auto calculated = reinterpret_cast<uintptr_t>(obj.get()) +
       nodedbg_offset_BaseObject__persistent_handle___v8_Persistent_v8_Object;
   EXPECT_EQ(expected, calculated);
-
-  obj.persistent().Reset();  // ~BaseObject() expects an empty handle.
 }