[api, heap] Implement TracedReference

TracedGlobalTrait was unable to override v8::TracedGlobal<v8::Object> for
avoiding the destructor because it is needed on the API surface itself and C++
ODR which prohibits specialization after template instantiation.

Avoid this problem by providing a separate type TracedReference
that, similar to TracedGlobal, is purely traced but avoids the destructor
completely. This only works for embedders that have their memory management
tied to V8 as it is prone to accessing already reclaimed objects otherwise.

Bug: chromium:995684
Change-Id: Iab4332ed417b26c58638a8f9389174cc355a305b
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1840972
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#64150}

src/api/api.cc

@@ -10589,11 +10589,12 @@ void EmbedderHeapTracer::DecreaseAllocatedSize(size_t bytes) {
 }
 
 void EmbedderHeapTracer::RegisterEmbedderReference(
-    const TracedGlobal<v8::Value>& ref) {
+    const TracedReferenceBase<v8::Value>& ref) {
   if (ref.IsEmpty()) return;
 
   i::Heap* const heap = reinterpret_cast<i::Isolate*>(isolate_)->heap();
-  heap->RegisterExternallyReferencedObject(reinterpret_cast<i::Address*>(*ref));
+  heap->RegisterExternallyReferencedObject(
+      reinterpret_cast<i::Address*>(ref.val_));
 }
 
 void EmbedderHeapTracer::IterateTracedGlobalHandles(
@@ -10603,6 +10604,26 @@ void EmbedderHeapTracer::IterateTracedGlobalHandles(
   isolate->global_handles()->IterateTracedNodes(visitor);
 }
 
+bool EmbedderHeapTracer::IsRootForNonTracingGC(
+    const v8::TracedReference<v8::Value>& handle) {
+  return true;
+}
+
+bool EmbedderHeapTracer::IsRootForNonTracingGC(
+    const v8::TracedGlobal<v8::Value>& handle) {
+  return true;
+}
+
+void EmbedderHeapTracer::ResetHandleInNonTracingGC(
+    const v8::TracedReference<v8::Value>& handle) {
+  UNREACHABLE();
+}
+
+void EmbedderHeapTracer::ResetHandleInNonTracingGC(
+    const v8::TracedGlobal<v8::Value>& handle) {
+  UNREACHABLE();
+}
+
 namespace internal {
 
 const size_t HandleScopeImplementer::kEnteredContextsOffset =

src/handles/global-handles.cc

@@ -901,8 +901,13 @@ void GlobalHandles::IdentifyWeakUnmodifiedObjects(
       DCHECK(node->is_root());
       if (is_unmodified(node->location())) {
         v8::Value* value = ToApi<v8::Value>(node->handle());
-        node->set_root(tracer->IsRootForNonTracingGC(
-            *reinterpret_cast<v8::TracedGlobal<v8::Value>*>(&value)));
+        if (node->has_destructor()) {
+          node->set_root(tracer->IsRootForNonTracingGC(
+              *reinterpret_cast<v8::TracedGlobal<v8::Value>*>(&value)));
+        } else {
+          node->set_root(tracer->IsRootForNonTracingGC(
+              *reinterpret_cast<v8::TracedReference<v8::Value>*>(&value)));
+        }
       }
     }
   }
@@ -990,7 +995,7 @@ void GlobalHandles::IterateYoungWeakUnmodifiedRootsForPhantomHandles(
         } else {
           v8::Value* value = ToApi<v8::Value>(node->handle());
           tracer->ResetHandleInNonTracingGC(
-              *reinterpret_cast<v8::TracedGlobal<v8::Value>*>(&value));
+              *reinterpret_cast<v8::TracedReference<v8::Value>*>(&value));
           DCHECK(!node->IsInUse());
         }
 
@@ -1271,8 +1276,13 @@ void GlobalHandles::IterateTracedNodes(
   for (TracedNode* node : *traced_nodes_) {
     if (node->IsInUse()) {
       v8::Value* value = ToApi<v8::Value>(node->handle());
-      visitor->VisitTracedGlobalHandle(
-          *reinterpret_cast<v8::TracedGlobal<v8::Value>*>(&value));
+      if (node->has_destructor()) {
+        visitor->VisitTracedGlobalHandle(
+            *reinterpret_cast<v8::TracedGlobal<v8::Value>*>(&value));
+      } else {
+        visitor->VisitTracedReference(
+            *reinterpret_cast<v8::TracedReference<v8::Value>*>(&value));
+      }
     }
   }
 }

src/heap/embedder-tracing.h

@@ -57,7 +57,12 @@ class V8_EXPORT_PRIVATE LocalEmbedderHeapTracer final {
   bool IsRootForNonTracingGC(const v8::TracedGlobal<v8::Value>& handle) {
     return !InUse() || remote_tracer_->IsRootForNonTracingGC(handle);
   }
-  void ResetHandleInNonTracingGC(const v8::TracedGlobal<v8::Value>& handle) {
+
+  bool IsRootForNonTracingGC(const v8::TracedReference<v8::Value>& handle) {
+    return !InUse() || remote_tracer_->IsRootForNonTracingGC(handle);
+  }
+
+  void ResetHandleInNonTracingGC(const v8::TracedReference<v8::Value>& handle) {
     // Resetting is only called when IsRootForNonTracingGC returns false which
     // can only happen the EmbedderHeapTracer is set on API level.
     DCHECK(InUse());

test/cctest/heap/test-embedder-tracing.cc

@@ -17,12 +17,6 @@
 
 namespace v8 {
 
-// See test below: TracedGlobalNoDestructor.
-template <>
-struct TracedGlobalTrait<v8::TracedGlobal<v8::Value>> {
-  static constexpr bool kRequiresExplicitDestruction = false;
-};
-
 namespace internal {
 namespace heap {
 
@@ -293,13 +287,14 @@ void ConstructJSObject(v8::Isolate* isolate, v8::Local<v8::Context> context,
   CHECK(!global->IsEmpty());
 }
 
+template <typename T>
 void ConstructJSApiObject(v8::Isolate* isolate, v8::Local<v8::Context> context,
-                          v8::TracedGlobal<v8::Object>* global) {
+                          T* global) {
   v8::HandleScope scope(isolate);
   v8::Local<v8::Object> object(
       ConstructTraceableJSApiObject(context, nullptr, nullptr));
   CHECK(!object.IsEmpty());
-  *global = v8::TracedGlobal<v8::Object>(isolate, object);
+  *global = T(isolate, object);
   CHECK(!global->IsEmpty());
 }
 
@@ -360,10 +355,6 @@ TEST(TracedGlobalCopyWithDestructor) {
   v8::HandleScope scope(isolate);
   i::GlobalHandles* global_handles = CcTest::i_isolate()->global_handles();
 
-  static_assert(TracedGlobalTrait<
-                    v8::TracedGlobal<v8::Object>>::kRequiresExplicitDestruction,
-                "destructor expected");
-
   const size_t initial_count = global_handles->handles_count();
   v8::TracedGlobal<v8::Object> global1;
   {
@@ -401,18 +392,14 @@ TEST(TracedGlobalCopyNoDestructor) {
   v8::HandleScope scope(isolate);
   i::GlobalHandles* global_handles = CcTest::i_isolate()->global_handles();
 
-  static_assert(!TracedGlobalTrait<
-                    v8::TracedGlobal<v8::Value>>::kRequiresExplicitDestruction,
-                "no destructor expected");
-
   const size_t initial_count = global_handles->handles_count();
-  v8::TracedGlobal<v8::Value> global1;
+  v8::TracedReference<v8::Value> global1;
   {
     v8::HandleScope scope(isolate);
     global1.Reset(isolate, v8::Object::New(isolate));
   }
-  v8::TracedGlobal<v8::Value> global2(global1);
-  v8::TracedGlobal<v8::Value> global3;
+  v8::TracedReference<v8::Value> global2(global1);
+  v8::TracedReference<v8::Value> global3;
   global3 = global2;
   CHECK_EQ(initial_count + 3, global_handles->handles_count());
   CHECK(!global1.IsEmpty());
@@ -500,7 +487,7 @@ TEST(TracedGlobalToUnmodifiedJSApiObjectSurvivesScavengePerDefault) {
   heap::TemporaryEmbedderHeapTracerScope tracer_scope(isolate, &tracer);
   tracer.ConsiderTracedGlobalAsRoot(true);
   TracedGlobalTest(
-      CcTest::isolate(), ConstructJSApiObject,
+      CcTest::isolate(), ConstructJSApiObject<TracedGlobal<v8::Object>>,
       [](const TracedGlobal<v8::Object>& global) {}, InvokeScavenge,
       SurvivalMode::kSurvives);
 }
@@ -513,7 +500,7 @@ TEST(TracedGlobalToUnmodifiedJSApiObjectDiesOnScavengeWhenExcludedFromRoots) {
   heap::TemporaryEmbedderHeapTracerScope tracer_scope(isolate, &tracer);
   tracer.ConsiderTracedGlobalAsRoot(false);
   TracedGlobalTest(
-      CcTest::isolate(), ConstructJSApiObject,
+      CcTest::isolate(), ConstructJSApiObject<TracedGlobal<v8::Object>>,
       [](const TracedGlobal<v8::Object>& global) {}, InvokeScavenge,
       SurvivalMode::kDies);
 }
@@ -671,9 +658,6 @@ TEST(TracedGlobalWithDestructor) {
     CHECK(!traced->IsEmpty());
     CHECK_EQ(initial_count + 1, global_handles->handles_count());
   }
-  static_assert(TracedGlobalTrait<
-                    v8::TracedGlobal<v8::Object>>::kRequiresExplicitDestruction,
-                "destructor expected");
   delete traced;
   CHECK_EQ(initial_count, global_handles->handles_count());
   // GC should not need to clear the handle.
@@ -691,21 +675,18 @@ TEST(TracedGlobalNoDestructor) {
   i::GlobalHandles* global_handles = CcTest::i_isolate()->global_handles();
 
   const size_t initial_count = global_handles->handles_count();
-  char* memory = new char[sizeof(v8::TracedGlobal<v8::Value>)];
-  auto* traced = new (memory) v8::TracedGlobal<v8::Value>();
+  char* memory = new char[sizeof(v8::TracedReference<v8::Value>)];
+  auto* traced = new (memory) v8::TracedReference<v8::Value>();
   {
     v8::HandleScope scope(isolate);
     v8::Local<v8::Value> object(ConstructTraceableJSApiObject(
         isolate->GetCurrentContext(), nullptr, nullptr));
     CHECK(traced->IsEmpty());
-    *traced = v8::TracedGlobal<v8::Value>(isolate, object);
+    *traced = v8::TracedReference<v8::Value>(isolate, object);
     CHECK(!traced->IsEmpty());
     CHECK_EQ(initial_count + 1, global_handles->handles_count());
   }
-  static_assert(!TracedGlobalTrait<
-                    v8::TracedGlobal<v8::Value>>::kRequiresExplicitDestruction,
-                "no destructor expected");
-  traced->~TracedGlobal<v8::Value>();
+  traced->~TracedReference<v8::Value>();
   CHECK_EQ(initial_count + 1, global_handles->handles_count());
   // GC should clear the handle.
   heap::InvokeMarkSweep();
@@ -759,18 +740,19 @@ class EmbedderHeapTracerNoDestructorNonTracingClearing final
       uint16_t class_id_to_optimize)
       : class_id_to_optimize_(class_id_to_optimize) {}
 
-  bool IsRootForNonTracingGC(const v8::TracedGlobal<v8::Value>& handle) final {
+  bool IsRootForNonTracingGC(
+      const v8::TracedReference<v8::Value>& handle) final {
     return handle.WrapperClassId() != class_id_to_optimize_;
   }
 
   void ResetHandleInNonTracingGC(
-      const v8::TracedGlobal<v8::Value>& handle) final {
+      const v8::TracedReference<v8::Value>& handle) final {
     if (handle.WrapperClassId() != class_id_to_optimize_) return;
 
     // Convention (for test): Objects that are optimized have their first field
     // set as a back pointer.
-    TracedGlobal<v8::Value>* original_handle =
-        reinterpret_cast<TracedGlobal<v8::Value>*>(
+    TracedReferenceBase<v8::Value>* original_handle =
+        reinterpret_cast<TracedReferenceBase<v8::Value>*>(
             v8::Object::GetAlignedPointerFromInternalField(
                 handle.As<v8::Object>(), 0));
     original_handle->Reset();
@@ -781,23 +763,23 @@ class EmbedderHeapTracerNoDestructorNonTracingClearing final
 };
 
 template <typename T>
-void SetupOptimizedAndNonOptimizedHandle(
-    v8::Isolate* isolate, uint16_t optimized_class_id,
-    v8::TracedGlobal<T>* optimized_handle,
-    v8::TracedGlobal<T>* non_optimized_handle) {
+void SetupOptimizedAndNonOptimizedHandle(v8::Isolate* isolate,
+                                         uint16_t optimized_class_id,
+                                         T* optimized_handle,
+                                         T* non_optimized_handle) {
   v8::HandleScope scope(isolate);
 
   v8::Local<v8::Object> optimized_object(ConstructTraceableJSApiObject(
       isolate->GetCurrentContext(), optimized_handle, nullptr));
   CHECK(optimized_handle->IsEmpty());
-  *optimized_handle = v8::TracedGlobal<T>(isolate, optimized_object);
+  *optimized_handle = T(isolate, optimized_object);
   CHECK(!optimized_handle->IsEmpty());
   optimized_handle->SetWrapperClassId(optimized_class_id);
 
   v8::Local<v8::Object> non_optimized_object(ConstructTraceableJSApiObject(
       isolate->GetCurrentContext(), nullptr, nullptr));
   CHECK(non_optimized_handle->IsEmpty());
-  *non_optimized_handle = v8::TracedGlobal<T>(isolate, non_optimized_object);
+  *non_optimized_handle = T(isolate, non_optimized_object);
   CHECK(!non_optimized_handle->IsEmpty());
 }
 
@@ -813,9 +795,6 @@ TEST(TracedGlobalDestructorReclaimedOnScavenge) {
   heap::TemporaryEmbedderHeapTracerScope tracer_scope(isolate, &tracer);
   i::GlobalHandles* global_handles = CcTest::i_isolate()->global_handles();
 
-  static_assert(TracedGlobalTrait<
-                    v8::TracedGlobal<v8::Object>>::kRequiresExplicitDestruction,
-                "destructor expected");
   const size_t initial_count = global_handles->handles_count();
   auto* optimized_handle = new v8::TracedGlobal<v8::Object>();
   auto* non_optimized_handle = new v8::TracedGlobal<v8::Object>();
@@ -841,12 +820,9 @@ TEST(TracedGlobalNoDestructorReclaimedOnScavenge) {
   heap::TemporaryEmbedderHeapTracerScope tracer_scope(isolate, &tracer);
   i::GlobalHandles* global_handles = CcTest::i_isolate()->global_handles();
 
-  static_assert(!TracedGlobalTrait<
-                    v8::TracedGlobal<v8::Value>>::kRequiresExplicitDestruction,
-                "no destructor expected");
   const size_t initial_count = global_handles->handles_count();
-  auto* optimized_handle = new v8::TracedGlobal<v8::Value>();
-  auto* non_optimized_handle = new v8::TracedGlobal<v8::Value>();
+  auto* optimized_handle = new v8::TracedReference<v8::Value>();
+  auto* non_optimized_handle = new v8::TracedReference<v8::Value>();
   SetupOptimizedAndNonOptimizedHandle(isolate, kClassIdToOptimize,
                                       optimized_handle, non_optimized_handle);
   CHECK_EQ(initial_count + 2, global_handles->handles_count());