Implement managed objects with phantom handles

For each Managed<T> (which is a Foreign), we create a weak global handle
with a finalizer which deletes the referenced C++ object once the
Foreign is dead.
Before calling this finalizer, the garbage collector needs to mark the
referenced object black (i.e. live), because the finalizer might
resurrect it.
Since this is never done for managed objects, we can use the more
lightweight phantom handle semantics, which allows the referenced
object to be garbage collected right away.

However, we can't access the global handle via the WeakCallbackInfo,
because the global handle will already be garbage collected. So we need
to store it explicitly. This is solved by storing the global handle
together with the finalizer.
In order to implement this, ownership of the ManagedObjectFinalizer
is moved from the isolate to the managed object.

R=ulan@chromium.org, mtrofin@chromium.org
BUG=v8:6505, chromium:734345

Change-Id: I94a245df601f70e19355d82439d30099e159231b
Reviewed-on: https://chromium-review.googlesource.com/539578
Commit-Queue: Clemens Hammacher <clemensh@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#46036}

src/isolate.cc

@@ -2091,42 +2091,36 @@ void Isolate::ReleaseManagedObjects() {
   while (current != nullptr) {
     Isolate::ManagedObjectFinalizer* next = current->next_;
     current->Dispose();
-    delete current;
     current = next;
   }
   // No new managed objects should pop up during finalization.
   DCHECK_NULL(managed_object_finalizers_list_.next_);
 }
 
-Isolate::ManagedObjectFinalizer* Isolate::RegisterForReleaseAtTeardown(
-    void* value, Isolate::ManagedObjectFinalizer::Deleter deleter) {
-  DCHECK_NOT_NULL(value);
-  DCHECK_NOT_NULL(deleter);
+void Isolate::RegisterForReleaseAtTeardown(
+    Isolate::ManagedObjectFinalizer* finalizer) {
+  DCHECK_NOT_NULL(finalizer->value_);
+  DCHECK_NOT_NULL(finalizer->deleter_);
+  DCHECK_NULL(finalizer->prev_);
+  DCHECK_NULL(finalizer->next_);
 
-  Isolate::ManagedObjectFinalizer* ret = new Isolate::ManagedObjectFinalizer();
-  ret->value_ = value;
-  ret->deleter_ = deleter;
   // Insert at head. We keep the head alive for the lifetime of the Isolate
   // because otherwise we can't reset the head, should we delete it before
   // the isolate expires
   Isolate::ManagedObjectFinalizer* next = managed_object_finalizers_list_.next_;
-  managed_object_finalizers_list_.next_ = ret;
-  ret->prev_ = &managed_object_finalizers_list_;
-  ret->next_ = next;
-  if (next != nullptr) next->prev_ = ret;
-  return ret;
+  managed_object_finalizers_list_.next_ = finalizer;
+  finalizer->prev_ = &managed_object_finalizers_list_;
+  finalizer->next_ = next;
+  if (next != nullptr) next->prev_ = finalizer;
 }
 
 void Isolate::UnregisterFromReleaseAtTeardown(
-    Isolate::ManagedObjectFinalizer** finalizer_ptr) {
-  DCHECK_NOT_NULL(finalizer_ptr);
-  Isolate::ManagedObjectFinalizer* finalizer = *finalizer_ptr;
+    Isolate::ManagedObjectFinalizer* finalizer) {
+  DCHECK_NOT_NULL(finalizer);
   DCHECK_NOT_NULL(finalizer->prev_);
 
   finalizer->prev_->next_ = finalizer->next_;
   if (finalizer->next_ != nullptr) finalizer->next_->prev_ = finalizer->prev_;
-  delete finalizer;
-  *finalizer_ptr = nullptr;
 }
 
 Isolate::PerIsolateThreadData::~PerIsolateThreadData() {

src/isolate.h

@@ -1273,36 +1273,39 @@ class Isolate {
 
   // List of native heap values allocated by the runtime as part of its
   // implementation that must be freed at isolate deinit.
-  class ManagedObjectFinalizer final {
+  class ManagedObjectFinalizer {
    public:
-    typedef void (*Deleter)(void*);
-    void Dispose() { deleter_(value_); }
+    using Deleter = void (*)(ManagedObjectFinalizer*);
 
-   private:
-    friend class Isolate;
-
-    ManagedObjectFinalizer() {
+    ManagedObjectFinalizer(void* value, Deleter deleter)
+        : value_(value), deleter_(deleter) {
       DCHECK_EQ(reinterpret_cast<void*>(this),
                 reinterpret_cast<void*>(&value_));
     }
 
-    // value_ must be the first member
+    void Dispose() { deleter_(this); }
+
+    void* value() const { return value_; }
+
+   private:
+    friend class Isolate;
+
+    ManagedObjectFinalizer() = default;
+
     void* value_ = nullptr;
     Deleter deleter_ = nullptr;
     ManagedObjectFinalizer* prev_ = nullptr;
     ManagedObjectFinalizer* next_ = nullptr;
   };
 
-  // Register a native value for destruction at isolate teardown.
-  ManagedObjectFinalizer* RegisterForReleaseAtTeardown(
-      void* value, ManagedObjectFinalizer::Deleter deleter);
+  // Register a finalizer to be called at isolate teardown.
+  void RegisterForReleaseAtTeardown(ManagedObjectFinalizer*);
 
   // Unregister a previously registered value from release at
-  // isolate teardown, deleting the ManagedObjectFinalizer.
+  // isolate teardown.
   // This transfers the responsibility of the previously managed value's
-  // deletion to the caller. Pass by pointer, because *finalizer_ptr gets
-  // reset to nullptr.
-  void UnregisterFromReleaseAtTeardown(ManagedObjectFinalizer** finalizer_ptr);
+  // deletion to the caller.
+  void UnregisterFromReleaseAtTeardown(ManagedObjectFinalizer*);
 
   size_t elements_deletion_counter() { return elements_deletion_counter_; }
   void set_elements_deletion_counter(size_t value) {

src/managed.h

@@ -20,11 +20,21 @@ namespace internal {
 // address is typed as CppType**. The double indirection is due to the
 // use, by Managed, of Isolate::ManagedObjectFinalizer, which has a CppType*
 // first field.
+// Calling Foreign::set_foreign_address is not allowed on a Managed object.
 template <class CppType>
 class Managed : public Foreign {
+  class FinalizerWithHandle : public Isolate::ManagedObjectFinalizer {
+   public:
+    FinalizerWithHandle(void* value,
+                        Isolate::ManagedObjectFinalizer::Deleter deleter)
+        : Isolate::ManagedObjectFinalizer(value, deleter) {}
+
+    Object** global_handle_location;
+  };
+
  public:
   V8_INLINE CppType* get() {
-    return *(reinterpret_cast<CppType**>(foreign_address()));
+    return reinterpret_cast<CppType*>(GetFinalizer()->value());
   }
 
   static Managed<CppType>* cast(Object* obj) {
@@ -33,46 +43,42 @@ class Managed : public Foreign {
   }
 
   static Handle<Managed<CppType>> New(Isolate* isolate, CppType* ptr) {
-    Isolate::ManagedObjectFinalizer* node =
-        isolate->RegisterForReleaseAtTeardown(ptr,
-                                              Managed<CppType>::NativeDelete);
+    FinalizerWithHandle* finalizer =
+        new FinalizerWithHandle(ptr, &NativeDelete);
+    isolate->RegisterForReleaseAtTeardown(finalizer);
     Handle<Managed<CppType>> handle = Handle<Managed<CppType>>::cast(
-        isolate->factory()->NewForeign(reinterpret_cast<Address>(node)));
-    RegisterWeakCallbackForDelete(isolate, handle);
+        isolate->factory()->NewForeign(reinterpret_cast<Address>(finalizer)));
+    Handle<Object> global_handle = isolate->global_handles()->Create(*handle);
+    finalizer->global_handle_location = global_handle.location();
+    GlobalHandles::MakeWeak(finalizer->global_handle_location,
+                            handle->GetFinalizer(), &Managed<CppType>::GCDelete,
+                            v8::WeakCallbackType::kParameter);
+
     return handle;
   }
 
  private:
-  static void RegisterWeakCallbackForDelete(Isolate* isolate,
-                                            Handle<Managed<CppType>> handle) {
-    Handle<Object> global_handle = isolate->global_handles()->Create(*handle);
-    GlobalHandles::MakeWeak(global_handle.location(), global_handle.location(),
-                            &Managed<CppType>::GCDelete,
-                            v8::WeakCallbackType::kFinalizer);
-  }
-
   static void GCDelete(const v8::WeakCallbackInfo<void>& data) {
-    Managed<CppType>** p =
-        reinterpret_cast<Managed<CppType>**>(data.GetParameter());
-
-    Isolate::ManagedObjectFinalizer* finalizer = (*p)->GetFinalizer();
+    FinalizerWithHandle* finalizer =
+        reinterpret_cast<FinalizerWithHandle*>(data.GetParameter());
 
     Isolate* isolate = reinterpret_cast<Isolate*>(data.GetIsolate());
-    finalizer->Dispose();
-    isolate->UnregisterFromReleaseAtTeardown(&finalizer);
+    isolate->UnregisterFromReleaseAtTeardown(finalizer);
 
-    (*p)->set_foreign_address(static_cast<Address>(nullptr));
-    GlobalHandles::Destroy(reinterpret_cast<Object**>(p));
+    GlobalHandles::Destroy(finalizer->global_handle_location);
+    NativeDelete(finalizer);
   }
 
-  static void NativeDelete(void* value) {
-    CppType* typed_value = reinterpret_cast<CppType*>(value);
+  static void NativeDelete(Isolate::ManagedObjectFinalizer* finalizer) {
+    CppType* typed_value = reinterpret_cast<CppType*>(finalizer->value());
     delete typed_value;
+    FinalizerWithHandle* finalizer_with_handle =
+        static_cast<FinalizerWithHandle*>(finalizer);
+    delete finalizer_with_handle;
   }
 
-  Isolate::ManagedObjectFinalizer* GetFinalizer() {
-    return reinterpret_cast<Isolate::ManagedObjectFinalizer*>(
-        foreign_address());
+  FinalizerWithHandle* GetFinalizer() {
+    return reinterpret_cast<FinalizerWithHandle*>(foreign_address());
   }
 };
 }  // namespace internal

test/cctest/test-managed.cc

@@ -20,8 +20,8 @@ class DeleteRecorder {
     *deleted_ = false;
   }
   ~DeleteRecorder() { *deleted_ = true; }
-  static void Deleter(void* value) {
-    delete reinterpret_cast<DeleteRecorder*>(value);
+  static void Deleter(Isolate::ManagedObjectFinalizer* finalizer) {
+    delete *reinterpret_cast<DeleteRecorder**>(finalizer);
   }
 
  private:
@@ -34,8 +34,8 @@ TEST(ManagedCollect) {
   bool deleted2 = false;
   DeleteRecorder* d1 = new DeleteRecorder(&deleted1);
   DeleteRecorder* d2 = new DeleteRecorder(&deleted2);
-  Isolate::ManagedObjectFinalizer* finalizer =
-      isolate->RegisterForReleaseAtTeardown(d2, DeleteRecorder::Deleter);
+  Isolate::ManagedObjectFinalizer finalizer(d2, DeleteRecorder::Deleter);
+  isolate->RegisterForReleaseAtTeardown(&finalizer);
   {
     HandleScope scope(isolate);
     auto handle = Managed<DeleteRecorder>::New(isolate, d1);
@@ -47,7 +47,6 @@ TEST(ManagedCollect) {
   CHECK(deleted1);
   CHECK(!deleted2);
   isolate->UnregisterFromReleaseAtTeardown(&finalizer);
-  CHECK_NULL(finalizer);
   delete d2;
   CHECK(deleted2);
 }
@@ -69,7 +68,8 @@ TEST(DisposeCollect) {
     auto handle = Managed<DeleteRecorder>::New(i_isolate, d1);
     USE(handle);
   }
-  i_isolate->RegisterForReleaseAtTeardown(d2, DeleteRecorder::Deleter);
+  Isolate::ManagedObjectFinalizer finalizer(d2, DeleteRecorder::Deleter);
+  i_isolate->RegisterForReleaseAtTeardown(&finalizer);
 
   isolate->Exit();
   isolate->Dispose();