Migrate CrossThreadPersistent

Adds a cross-thread reference for strongly and weakly retaining
objects on a thread other than the thread that owns the object.

The intended use of the reference is by setting it up on the
originating thread, holding the object alive from another thread, and
ultimately accessing the object again on the originating thread.

The reference has known caveats:
- It's unsafe to use when the heap may terminate;
- It's unsafe to transitively reach through the graph because of
  compaction;

Change-Id: I84fbdde69a099eb54af5b93c34e2169915b17e64
Bug: chromium:1056170
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2436449
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Omer Katz <omerkatz@chromium.org>
Reviewed-by: Anton Bikineev <bikineev@chromium.org>
Cr-Commit-Position: refs/heads/master@{#70428}

BUILD.gn

@@ -4422,6 +4422,7 @@ v8_source_set("cppgc_base") {
     "src/heap/cppgc/prefinalizer-handler.cc",
     "src/heap/cppgc/prefinalizer-handler.h",
     "src/heap/cppgc/process-heap.cc",
+    "src/heap/cppgc/process-heap.h",
     "src/heap/cppgc/raw-heap.cc",
     "src/heap/cppgc/raw-heap.h",
     "src/heap/cppgc/sanitizers.h",

include/cppgc/cross-thread-persistent.h

@@ -0,0 +1,311 @@
+// Copyright 2020 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef INCLUDE_CPPGC_CROSS_THREAD_PERSISTENT_H_
+#define INCLUDE_CPPGC_CROSS_THREAD_PERSISTENT_H_
+
+#include <atomic>
+
+#include "cppgc/internal/persistent-node.h"
+#include "cppgc/internal/pointer-policies.h"
+#include "cppgc/persistent.h"
+#include "cppgc/visitor.h"
+
+namespace cppgc {
+
+namespace internal {
+
+template <typename T, typename WeaknessPolicy, typename LocationPolicy,
+          typename CheckingPolicy>
+class BasicCrossThreadPersistent final : public PersistentBase,
+                                         public LocationPolicy,
+                                         private WeaknessPolicy,
+                                         private CheckingPolicy {
+ public:
+  using typename WeaknessPolicy::IsStrongPersistent;
+  using PointeeType = T;
+
+  ~BasicCrossThreadPersistent() { Clear(); }
+
+  BasicCrossThreadPersistent(  // NOLINT
+      const SourceLocation& loc = SourceLocation::Current())
+      : LocationPolicy(loc) {}
+
+  BasicCrossThreadPersistent(  // NOLINT
+      std::nullptr_t, const SourceLocation& loc = SourceLocation::Current())
+      : LocationPolicy(loc) {}
+
+  BasicCrossThreadPersistent(  // NOLINT
+      SentinelPointer s, const SourceLocation& loc = SourceLocation::Current())
+      : PersistentBase(s), LocationPolicy(loc) {}
+
+  BasicCrossThreadPersistent(  // NOLINT
+      T* raw, const SourceLocation& loc = SourceLocation::Current())
+      : PersistentBase(raw), LocationPolicy(loc) {
+    if (!IsValid(raw)) return;
+    PersistentRegionLock guard;
+    PersistentRegion& region = this->GetPersistentRegion(raw);
+    SetNode(region.AllocateNode(this, &Trace));
+    this->CheckPointer(raw);
+  }
+
+  BasicCrossThreadPersistent(  // NOLINT
+      T& raw, const SourceLocation& loc = SourceLocation::Current())
+      : BasicCrossThreadPersistent(&raw, loc) {}
+
+  template <typename U, typename MemberBarrierPolicy,
+            typename MemberWeaknessTag, typename MemberCheckingPolicy,
+            typename = std::enable_if_t<std::is_base_of<T, U>::value>>
+  BasicCrossThreadPersistent(  // NOLINT
+      internal::BasicMember<U, MemberBarrierPolicy, MemberWeaknessTag,
+                            MemberCheckingPolicy>
+          member,
+      const SourceLocation& loc = SourceLocation::Current())
+      : BasicCrossThreadPersistent(member.Get(), loc) {}
+
+  BasicCrossThreadPersistent(
+      const BasicCrossThreadPersistent& other,
+      const SourceLocation& loc = SourceLocation::Current())
+      : BasicCrossThreadPersistent(loc) {
+    // Invoke operator=.
+    *this = other;
+  }
+
+  // Heterogeneous ctor.
+  template <typename U, typename OtherWeaknessPolicy,
+            typename OtherLocationPolicy, typename OtherCheckingPolicy,
+            typename = std::enable_if_t<std::is_base_of<T, U>::value>>
+  BasicCrossThreadPersistent(  // NOLINT
+      const BasicCrossThreadPersistent<U, OtherWeaknessPolicy,
+                                       OtherLocationPolicy,
+                                       OtherCheckingPolicy>& other,
+      const SourceLocation& loc = SourceLocation::Current())
+      : BasicCrossThreadPersistent(loc) {
+    *this = other;
+  }
+
+  BasicCrossThreadPersistent(
+      BasicCrossThreadPersistent&& other,
+      const SourceLocation& loc = SourceLocation::Current()) noexcept {
+    // Invoke operator=.
+    *this = std::move(other);
+  }
+
+  BasicCrossThreadPersistent& operator=(
+      const BasicCrossThreadPersistent& other) {
+    PersistentRegionLock guard;
+    AssignUnsafe(other.Get());
+    return *this;
+  }
+
+  template <typename U, typename OtherWeaknessPolicy,
+            typename OtherLocationPolicy, typename OtherCheckingPolicy,
+            typename = std::enable_if_t<std::is_base_of<T, U>::value>>
+  BasicCrossThreadPersistent& operator=(
+      const BasicCrossThreadPersistent<U, OtherWeaknessPolicy,
+                                       OtherLocationPolicy,
+                                       OtherCheckingPolicy>& other) {
+    PersistentRegionLock guard;
+    AssignUnsafe(other.Get());
+    return *this;
+  }
+
+  BasicCrossThreadPersistent& operator=(BasicCrossThreadPersistent&& other) {
+    if (this == &other) return *this;
+    Clear();
+    PersistentRegionLock guard;
+    PersistentBase::operator=(std::move(other));
+    LocationPolicy::operator=(std::move(other));
+    if (!IsValid(GetValue())) return *this;
+    GetNode()->UpdateOwner(this);
+    other.SetValue(nullptr);
+    other.SetNode(nullptr);
+    this->CheckPointer(GetValue());
+    return *this;
+  }
+
+  BasicCrossThreadPersistent& operator=(T* other) {
+    Assign(other);
+    return *this;
+  }
+
+  // Assignment from member.
+  template <typename U, typename MemberBarrierPolicy,
+            typename MemberWeaknessTag, typename MemberCheckingPolicy,
+            typename = std::enable_if_t<std::is_base_of<T, U>::value>>
+  BasicCrossThreadPersistent& operator=(
+      internal::BasicMember<U, MemberBarrierPolicy, MemberWeaknessTag,
+                            MemberCheckingPolicy>
+          member) {
+    return operator=(member.Get());
+  }
+
+  BasicCrossThreadPersistent& operator=(std::nullptr_t) {
+    Clear();
+    return *this;
+  }
+
+  BasicCrossThreadPersistent& operator=(SentinelPointer s) {
+    Assign(s);
+    return *this;
+  }
+
+  /**
+   * Returns a pointer to the stored object.
+   *
+   * Note: **Not thread-safe.**
+   *
+   * \returns a pointer to the stored object.
+   */
+  // CFI cast exemption to allow passing SentinelPointer through T* and support
+  // heterogeneous assignments between different Member and Persistent handles
+  // based on their actual types.
+  V8_CLANG_NO_SANITIZE("cfi-unrelated-cast") T* Get() const {
+    return static_cast<T*>(GetValue());
+  }
+
+  /**
+   * Clears the stored object.
+   */
+  void Clear() { Assign(nullptr); }
+
+  /**
+   * Returns a pointer to the stored object and releases it.
+   *
+   * Note: **Not thread-safe.**
+   *
+   * \returns a pointer to the stored object.
+   */
+  T* Release() {
+    T* result = Get();
+    Clear();
+    return result;
+  }
+
+  /**
+   * Conversio to boolean.
+   *
+   * Note: **Not thread-safe.**
+   *
+   * \returns true if an actual object has been stored and false otherwise.
+   */
+  explicit operator bool() const { return Get(); }
+
+  /**
+   * Conversion to object of type T.
+   *
+   * Note: **Not thread-safe.**
+   *
+   * \returns the object.
+   */
+  operator T*() const { return Get(); }  // NOLINT
+
+  /**
+   * Dereferences the stored object.
+   *
+   * Note: **Not thread-safe.**
+   */
+  T* operator->() const { return Get(); }
+  T& operator*() const { return *Get(); }
+
+ private:
+  static bool IsValid(void* ptr) { return ptr && ptr != kSentinelPointer; }
+
+  static void Trace(Visitor* v, const void* ptr) {
+    const auto* handle = static_cast<const BasicCrossThreadPersistent*>(ptr);
+    v->TraceRoot(*handle, handle->Location());
+  }
+
+  void Assign(T* ptr) {
+    void* old_value = GetValue();
+    if (IsValid(old_value)) {
+      PersistentRegionLock guard;
+      PersistentRegion& region = this->GetPersistentRegion(old_value);
+      if (IsValid(ptr) && (&region == &this->GetPersistentRegion(ptr))) {
+        SetValue(ptr);
+        this->CheckPointer(ptr);
+        return;
+      }
+      region.FreeNode(GetNode());
+      SetNode(nullptr);
+    }
+    SetValue(ptr);
+    if (!IsValid(ptr)) return;
+    PersistentRegionLock guard;
+    SetNode(this->GetPersistentRegion(ptr).AllocateNode(this, &Trace));
+    this->CheckPointer(ptr);
+  }
+
+  void AssignUnsafe(T* ptr) {
+    void* old_value = GetValue();
+    if (IsValid(old_value)) {
+      PersistentRegion& region = this->GetPersistentRegion(old_value);
+      if (IsValid(ptr) && (&region == &this->GetPersistentRegion(ptr))) {
+        SetValue(ptr);
+        this->CheckPointer(ptr);
+        return;
+      }
+      region.FreeNode(GetNode());
+      SetNode(nullptr);
+    }
+    SetValue(ptr);
+    if (!IsValid(ptr)) return;
+    SetNode(this->GetPersistentRegion(ptr).AllocateNode(this, &Trace));
+    this->CheckPointer(ptr);
+  }
+
+  void ClearFromGC() const {
+    if (IsValid(GetValue())) {
+      WeaknessPolicy::GetPersistentRegion(GetValue()).FreeNode(GetNode());
+      PersistentBase::ClearFromGC();
+    }
+  }
+
+  friend class cppgc::Visitor;
+};
+
+template <typename T, typename LocationPolicy, typename CheckingPolicy>
+struct IsWeak<
+    BasicCrossThreadPersistent<T, internal::WeakCrossThreadPersistentPolicy,
+                               LocationPolicy, CheckingPolicy>>
+    : std::true_type {};
+
+}  // namespace internal
+
+namespace subtle {
+
+/**
+ * **DO NOT USE: Has known caveats, see below.**
+ *
+ * CrossThreadPersistent allows retaining objects from threads other than the
+ * thread the owning heap is operating on.
+ *
+ * Known caveats:
+ * - Does not protect the heap owning an object from terminating.
+ * - Reaching transitively through the graph is unsupported as objects may be
+ *   moved concurrently on the thread owning the object.
+ */
+template <typename T>
+using CrossThreadPersistent = internal::BasicCrossThreadPersistent<
+    T, internal::StrongCrossThreadPersistentPolicy>;
+
+/**
+ * **DO NOT USE: Has known caveats, see below.**
+ *
+ * CrossThreadPersistent allows weakly retaining objects from threads other than
+ * the thread the owning heap is operating on.
+ *
+ * Known caveats:
+ * - Does not protect the heap owning an object from terminating.
+ * - Reaching transitively through the graph is unsupported as objects may be
+ *   moved concurrently on the thread owning the object.
+ */
+template <typename T>
+using WeakCrossThreadPersistent = internal::BasicCrossThreadPersistent<
+    T, internal::WeakCrossThreadPersistentPolicy>;
+
+}  // namespace subtle
+}  // namespace cppgc
+
+#endif  // INCLUDE_CPPGC_CROSS_THREAD_PERSISTENT_H_

include/cppgc/internal/persistent-node.h

@@ -109,6 +109,14 @@ class V8_EXPORT PersistentRegion final {
   PersistentNode* free_list_head_ = nullptr;
 };
 
+// CrossThreadPersistent uses PersistentRegion but protects it using this lock
+// when needed.
+class V8_EXPORT PersistentRegionLock final {
+ public:
+  PersistentRegionLock();
+  ~PersistentRegionLock();
+};
+
 }  // namespace internal
 
 }  // namespace cppgc

include/cppgc/internal/pointer-policies.h

@@ -62,6 +62,7 @@ class KeepLocationPolicy {
   constexpr const SourceLocation& Location() const { return location_; }
 
  protected:
+  constexpr KeepLocationPolicy() = default;
   constexpr explicit KeepLocationPolicy(const SourceLocation& location)
       : location_(location) {}
 
@@ -82,6 +83,7 @@ class IgnoreLocationPolicy {
   constexpr SourceLocation Location() const { return {}; }
 
  protected:
+  constexpr IgnoreLocationPolicy() = default;
   constexpr explicit IgnoreLocationPolicy(const SourceLocation&) {}
 };
 
@@ -93,17 +95,29 @@ using DefaultLocationPolicy = IgnoreLocationPolicy;
 
 struct StrongPersistentPolicy {
   using IsStrongPersistent = std::true_type;
-
   static V8_EXPORT PersistentRegion& GetPersistentRegion(void* object);
 };
 
 struct WeakPersistentPolicy {
   using IsStrongPersistent = std::false_type;
-
   static V8_EXPORT PersistentRegion& GetPersistentRegion(void* object);
 };
 
-// Persistent/Member forward declarations.
+struct StrongCrossThreadPersistentPolicy {
+  using IsStrongPersistent = std::true_type;
+  static V8_EXPORT PersistentRegion& GetPersistentRegion(void* object);
+};
+
+struct WeakCrossThreadPersistentPolicy {
+  using IsStrongPersistent = std::false_type;
+  static V8_EXPORT PersistentRegion& GetPersistentRegion(void* object);
+};
+
+// Forward declarations setting up the default policies.
+template <typename T, typename WeaknessPolicy,
+          typename LocationPolicy = DefaultLocationPolicy,
+          typename CheckingPolicy = DisabledCheckingPolicy>
+class BasicCrossThreadPersistent;
 template <typename T, typename WeaknessPolicy,
           typename LocationPolicy = DefaultLocationPolicy,
           typename CheckingPolicy = DefaultCheckingPolicy>

include/cppgc/visitor.h

@@ -16,6 +16,9 @@
 namespace cppgc {
 
 namespace internal {
+template <typename T, typename WeaknessPolicy, typename LocationPolicy,
+          typename CheckingPolicy>
+class BasicCrossThreadPersistent;
 template <typename T, typename WeaknessPolicy, typename LocationPolicy,
           typename CheckingPolicy>
 class BasicPersistent;
@@ -200,6 +203,9 @@ class Visitor {
   V8_EXPORT void CheckObjectNotInConstruction(const void* address);
 #endif  // V8_ENABLE_CHECKS
 
+  template <typename T, typename WeaknessPolicy, typename LocationPolicy,
+            typename CheckingPolicy>
+  friend class internal::BasicCrossThreadPersistent;
   template <typename T, typename WeaknessPolicy, typename LocationPolicy,
             typename CheckingPolicy>
   friend class internal::BasicPersistent;

src/heap/cppgc/heap-base.h

@@ -113,6 +113,18 @@ class V8_EXPORT_PRIVATE HeapBase {
   const PersistentRegion& GetWeakPersistentRegion() const {
     return weak_persistent_region_;
   }
+  PersistentRegion& GetStrongCrossThreadPersistentRegion() {
+    return strong_cross_thread_persistent_region_;
+  }
+  const PersistentRegion& GetStrongCrossThreadPersistentRegion() const {
+    return strong_cross_thread_persistent_region_;
+  }
+  PersistentRegion& GetWeakCrossThreadPersistentRegion() {
+    return weak_cross_thread_persistent_region_;
+  }
+  const PersistentRegion& GetWeakCrossThreadPersistentRegion() const {
+    return weak_cross_thread_persistent_region_;
+  }
 
 #if defined(CPPGC_YOUNG_GENERATION)
   std::set<void*>& remembered_slots() { return remembered_slots_; }
@@ -149,6 +161,8 @@ class V8_EXPORT_PRIVATE HeapBase {
 
   PersistentRegion strong_persistent_region_;
   PersistentRegion weak_persistent_region_;
+  PersistentRegion strong_cross_thread_persistent_region_;
+  PersistentRegion weak_cross_thread_persistent_region_;
 
 #if defined(CPPGC_YOUNG_GENERATION)
   std::set<void*> remembered_slots_;

src/heap/cppgc/marker.cc

@@ -15,6 +15,7 @@
 #include "src/heap/cppgc/liveness-broker.h"
 #include "src/heap/cppgc/marking-state.h"
 #include "src/heap/cppgc/marking-visitor.h"
+#include "src/heap/cppgc/process-heap.h"
 #include "src/heap/cppgc/stats-collector.h"
 
 #if defined(CPPGC_CAGED_HEAP)
@@ -203,6 +204,12 @@ void MarkerBase::EnterAtomicPause(MarkingConfig::StackState stack_state) {
   config_.stack_state = stack_state;
   config_.marking_type = MarkingConfig::MarkingType::kAtomic;
 
+  // Lock guards against changes to {Weak}CrossThreadPersistent handles, that
+  // may conflict with marking. E.g., a WeakCrossThreadPersistent may be
+  // converted into a CrossThreadPersistent which requires that the handle
+  // is either cleared or the object is retained.
+  g_process_mutex.Pointer()->Lock();
+
   // VisitRoots also resets the LABs.
   VisitRoots(config_.stack_state);
   if (config_.stack_state == MarkingConfig::StackState::kNoHeapPointers) {
@@ -220,6 +227,7 @@ void MarkerBase::LeaveAtomicPause() {
       schedule_.GetOverallMarkedBytes());
   is_marking_started_ = false;
   ProcessWeakness();
+  g_process_mutex.Pointer()->Unlock();
 }
 
 void MarkerBase::FinishMarking(MarkingConfig::StackState stack_state) {
@@ -232,7 +240,11 @@ void MarkerBase::FinishMarking(MarkingConfig::StackState stack_state) {
 }
 
 void MarkerBase::ProcessWeakness() {
+  DCHECK_EQ(MarkingConfig::MarkingType::kAtomic, config_.marking_type);
   heap().GetWeakPersistentRegion().Trace(&visitor());
+  // Processing cross-thread handles requires taking the process lock.
+  g_process_mutex.Get().AssertHeld();
+  heap().GetWeakCrossThreadPersistentRegion().Trace(&visitor());
 
   // Call weak callbacks on objects that may now be pointing to dead objects.
   MarkingWorklists::WeakCallbackItem item;
@@ -252,6 +264,10 @@ void MarkerBase::VisitRoots(MarkingConfig::StackState stack_state) {
   heap().object_allocator().ResetLinearAllocationBuffers();
 
   heap().GetStrongPersistentRegion().Trace(&visitor());
+  if (config_.marking_type == MarkingConfig::MarkingType::kAtomic) {
+    g_process_mutex.Get().AssertHeld();
+    heap().GetStrongCrossThreadPersistentRegion().Trace(&visitor());
+  }
   if (stack_state != MarkingConfig::StackState::kNoHeapPointers) {
     heap().stack()->IteratePointers(&stack_visitor());
   }

src/heap/cppgc/persistent-node.cc

@@ -8,6 +8,7 @@
 #include <numeric>
 
 #include "include/cppgc/persistent.h"
+#include "src/heap/cppgc/process-heap.h"
 
 namespace cppgc {
 namespace internal {
@@ -68,5 +69,13 @@ void PersistentRegion::Trace(Visitor* visitor) {
                nodes_.end());
 }
 
+PersistentRegionLock::PersistentRegionLock() {
+  g_process_mutex.Pointer()->Lock();
+}
+
+PersistentRegionLock::~PersistentRegionLock() {
+  g_process_mutex.Pointer()->Unlock();
+}
+
 }  // namespace internal
 }  // namespace cppgc

src/heap/cppgc/pointer-policies.cc

@@ -31,5 +31,17 @@ PersistentRegion& WeakPersistentPolicy::GetPersistentRegion(void* object) {
   return heap->GetWeakPersistentRegion();
 }
 
+PersistentRegion& StrongCrossThreadPersistentPolicy::GetPersistentRegion(
+    void* object) {
+  auto* heap = BasePage::FromPayload(object)->heap();
+  return heap->GetStrongCrossThreadPersistentRegion();
+}
+
+PersistentRegion& WeakCrossThreadPersistentPolicy::GetPersistentRegion(
+    void* object) {
+  auto* heap = BasePage::FromPayload(object)->heap();
+  return heap->GetWeakCrossThreadPersistentRegion();
+}
+
 }  // namespace internal
 }  // namespace cppgc

src/heap/cppgc/process-heap.cc

@@ -2,6 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+#include "src/heap/cppgc/process-heap.h"
+
 #include "include/cppgc/internal/process-heap.h"
 
 namespace cppgc {
@@ -9,5 +11,7 @@ namespace internal {
 
 AtomicEntryFlag ProcessHeap::concurrent_marking_flag_;
 
+v8::base::LazyMutex g_process_mutex = LAZY_MUTEX_INITIALIZER;
+
 }  // namespace internal
 }  // namespace cppgc

src/heap/cppgc/process-heap.h

@@ -0,0 +1,18 @@
+// Copyright 2020 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#ifndef V8_HEAP_CPPGC_PROCESS_HEAP_H_
+#define V8_HEAP_CPPGC_PROCESS_HEAP_H_
+
+#include "src/base/platform/mutex.h"
+
+namespace cppgc {
+namespace internal {
+
+extern v8::base::LazyMutex g_process_mutex;
+
+}  // namespace internal
+}  // namespace cppgc
+
+#endif  // V8_HEAP_CPPGC_PROCESS_HEAP_H_

test/unittests/BUILD.gn

@@ -82,6 +82,7 @@ v8_source_set("cppgc_unittests_sources") {
   sources = [
     "heap/cppgc/concurrent-marking-unittest.cc",
     "heap/cppgc/concurrent-sweeper-unittest.cc",
+    "heap/cppgc/cross-thread-persistent-unittest.cc",
     "heap/cppgc/custom-spaces-unittest.cc",
     "heap/cppgc/finalizer-trait-unittest.cc",
     "heap/cppgc/free-list-unittest.cc",
@@ -102,7 +103,7 @@ v8_source_set("cppgc_unittests_sources") {
     "heap/cppgc/name-trait-unittest.cc",
     "heap/cppgc/object-start-bitmap-unittest.cc",
     "heap/cppgc/page-memory-unittest.cc",
-    "heap/cppgc/persistent-unittest.cc",
+    "heap/cppgc/persistent-family-unittest.cc",
     "heap/cppgc/prefinalizer-unittest.cc",
     "heap/cppgc/source-location-unittest.cc",
     "heap/cppgc/stack-unittest.cc",

test/unittests/heap/cppgc/cross-thread-persistent-unittest.cc

@@ -0,0 +1,101 @@
+// Copyright 2020 the V8 project authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "include/cppgc/cross-thread-persistent.h"
+
+#include "include/cppgc/allocation.h"
+#include "src/base/platform/condition-variable.h"
+#include "src/base/platform/mutex.h"
+#include "src/base/platform/platform.h"
+#include "test/unittests/heap/cppgc/tests.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
+namespace cppgc {
+namespace internal {
+
+namespace {
+
+struct GCed final : GarbageCollected<GCed> {
+  static size_t destructor_call_count;
+  GCed() { destructor_call_count = 0; }
+  ~GCed() { destructor_call_count++; }
+  virtual void Trace(cppgc::Visitor*) const {}
+  int a = 0;
+};
+size_t GCed::destructor_call_count = 0;
+
+class Runner final : public v8::base::Thread {
+ public:
+  template <typename Callback>
+  explicit Runner(Callback callback)
+      : Thread(v8::base::Thread::Options("CrossThreadPersistent Thread")),
+        callback_(callback) {}
+
+  void Run() final { callback_(); }
+
+ private:
+  std::function<void()> callback_;
+};
+
+}  // namespace
+
+class CrossThreadPersistentTest : public testing::TestWithHeap {};
+
+TEST_F(CrossThreadPersistentTest, RetainStronglyOnDifferentThread) {
+  subtle::CrossThreadPersistent<GCed> holder =
+      MakeGarbageCollected<GCed>(GetAllocationHandle());
+  {
+    Runner runner([obj = std::move(holder)]() {});
+    EXPECT_FALSE(holder);
+    EXPECT_EQ(0u, GCed::destructor_call_count);
+    PreciseGC();
+    EXPECT_EQ(0u, GCed::destructor_call_count);
+    runner.StartSynchronously();
+    runner.Join();
+  }
+  EXPECT_EQ(0u, GCed::destructor_call_count);
+  PreciseGC();
+  EXPECT_EQ(1u, GCed::destructor_call_count);
+}
+
+TEST_F(CrossThreadPersistentTest, RetainWeaklyOnDifferentThread) {
+  subtle::WeakCrossThreadPersistent<GCed> in =
+      MakeGarbageCollected<GCed>(GetAllocationHandle());
+  // Set up |out| with an object that is always retained to ensure that the
+  // different thread indeed moves back an empty handle.
+  Persistent<GCed> out_holder =
+      MakeGarbageCollected<GCed>(GetAllocationHandle());
+  subtle::WeakCrossThreadPersistent<GCed> out = *out_holder;
+  {
+    Persistent<GCed> temporary_holder = *in;
+    Runner runner([obj = std::move(in), &out]() { out = std::move(obj); });
+    EXPECT_FALSE(in);
+    EXPECT_TRUE(out);
+    EXPECT_EQ(0u, GCed::destructor_call_count);
+    PreciseGC();
+    EXPECT_EQ(0u, GCed::destructor_call_count);
+    temporary_holder.Clear();
+    PreciseGC();
+    EXPECT_EQ(1u, GCed::destructor_call_count);
+    runner.StartSynchronously();
+    runner.Join();
+  }
+  EXPECT_FALSE(out);
+}
+
+TEST_F(CrossThreadPersistentTest, DestroyRacingWithGC) {
+  // Destroy a handle on a different thread while at the same time invoking a
+  // garbage collection on the original thread.
+  subtle::CrossThreadPersistent<GCed> holder =
+      MakeGarbageCollected<GCed>(GetAllocationHandle());
+  Runner runner([&obj = holder]() { obj.Clear(); });
+  EXPECT_TRUE(holder);
+  runner.StartSynchronously();
+  PreciseGC();
+  runner.Join();
+  EXPECT_FALSE(holder);
+}
+
+}  // namespace internal
+}  // namespace cppgc

test/unittests/heap/cppgc/persistent-family-unittest.cc

@@ -2,14 +2,14 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "include/cppgc/persistent.h"
-
 #include <vector>
 
 #include "include/cppgc/allocation.h"
+#include "include/cppgc/cross-thread-persistent.h"
 #include "include/cppgc/garbage-collected.h"
 #include "include/cppgc/internal/pointer-policies.h"
 #include "include/cppgc/member.h"
+#include "include/cppgc/persistent.h"
 #include "include/cppgc/type-traits.h"
 #include "src/heap/cppgc/heap.h"
 #include "src/heap/cppgc/liveness-broker.h"
@@ -32,12 +32,40 @@ struct DerivedGCed : GCed {
   void Trace(cppgc::Visitor* v) const override { GCed::Trace(v); }
 };
 
+template <template <typename> class PersistentType>
+struct PersistentRegionTrait;
+
+template <>
+struct PersistentRegionTrait<Persistent> {
+  static PersistentRegion& Get(cppgc::Heap* heap) {
+    return internal::Heap::From(heap)->GetStrongPersistentRegion();
+  }
+};
+
+template <>
+struct PersistentRegionTrait<WeakPersistent> {
+  static PersistentRegion& Get(cppgc::Heap* heap) {
+    return internal::Heap::From(heap)->GetWeakPersistentRegion();
+  }
+};
+
+template <>
+struct PersistentRegionTrait<subtle::CrossThreadPersistent> {
+  static PersistentRegion& Get(cppgc::Heap* heap) {
+    return internal::Heap::From(heap)->GetStrongCrossThreadPersistentRegion();
+  }
+};
+
+template <>
+struct PersistentRegionTrait<subtle::WeakCrossThreadPersistent> {
+  static PersistentRegion& Get(cppgc::Heap* heap) {
+    return internal::Heap::From(heap)->GetWeakCrossThreadPersistentRegion();
+  }
+};
+
 template <template <typename> class PersistentType>
 PersistentRegion& GetRegion(cppgc::Heap* heap) {
-  auto* heap_impl = internal::Heap::From(heap);
-  return IsWeak<PersistentType<GCed>>::value
-             ? heap_impl->GetWeakPersistentRegion()
-             : heap_impl->GetStrongPersistentRegion();
+  return PersistentRegionTrait<PersistentType>::Get(heap);
 }
 
 template <typename T>
@@ -46,6 +74,11 @@ using LocalizedPersistent =
                               internal::KeepLocationPolicy,
                               internal::DefaultCheckingPolicy>;
 
+template <typename T>
+using LocalizedCrossThreadPersistent = internal::BasicCrossThreadPersistent<
+    T, internal::StrongCrossThreadPersistentPolicy,
+    internal::KeepLocationPolicy, internal::DefaultCheckingPolicy>;
+
 class RootVisitor final : public VisitorBase {
  public:
   RootVisitor() = default;
@@ -112,6 +145,8 @@ TEST_F(PersistentTest, NullStateCtor) {
   auto* heap = GetHeap();
   NullStateCtor<Persistent>(heap);
   NullStateCtor<WeakPersistent>(heap);
+  NullStateCtor<subtle::CrossThreadPersistent>(heap);
+  NullStateCtor<subtle::WeakCrossThreadPersistent>(heap);
 }
 
 template <template <typename> class PersistentType>
@@ -136,6 +171,8 @@ TEST_F(PersistentTest, RawCtor) {
   auto* heap = GetHeap();
   RawCtor<Persistent>(heap);
   RawCtor<WeakPersistent>(heap);
+  RawCtor<subtle::CrossThreadPersistent>(heap);
+  RawCtor<subtle::WeakCrossThreadPersistent>(heap);
 }
 
 template <template <typename> class PersistentType>
@@ -191,6 +228,8 @@ TEST_F(PersistentTest, CopyCtor) {
   auto* heap = GetHeap();
   CopyCtor<Persistent>(heap);
   CopyCtor<WeakPersistent>(heap);
+  CopyCtor<subtle::CrossThreadPersistent>(heap);
+  CopyCtor<subtle::WeakCrossThreadPersistent>(heap);
 }
 
 template <template <typename> class PersistentType>
@@ -233,6 +272,8 @@ TEST_F(PersistentTest, MoveCtor) {
   auto* heap = GetHeap();
   MoveCtor<Persistent>(heap);
   MoveCtor<WeakPersistent>(heap);
+  MoveCtor<subtle::CrossThreadPersistent>(heap);
+  MoveCtor<subtle::WeakCrossThreadPersistent>(heap);
 }
 
 template <template <typename> class PersistentType,
@@ -258,10 +299,16 @@ TEST_F(PersistentTest, MemberCtor) {
   MemberCtor<WeakPersistent, Member>(heap);
   MemberCtor<WeakPersistent, WeakMember>(heap);
   MemberCtor<WeakPersistent, UntracedMember>(heap);
+  MemberCtor<subtle::CrossThreadPersistent, Member>(heap);
+  MemberCtor<subtle::CrossThreadPersistent, WeakMember>(heap);
+  MemberCtor<subtle::CrossThreadPersistent, UntracedMember>(heap);
+  MemberCtor<subtle::WeakCrossThreadPersistent, Member>(heap);
+  MemberCtor<subtle::WeakCrossThreadPersistent, WeakMember>(heap);
+  MemberCtor<subtle::WeakCrossThreadPersistent, UntracedMember>(heap);
 }
 
 template <template <typename> class PersistentType>
-void NullStateAssignemnt(cppgc::Heap* heap) {
+void NullStateAssignment(cppgc::Heap* heap) {
   EXPECT_EQ(0u, GetRegion<PersistentType>(heap).NodesInUse());
   {
     PersistentType<GCed> p =
@@ -292,8 +339,10 @@ void NullStateAssignemnt(cppgc::Heap* heap) {
 
 TEST_F(PersistentTest, NullStateAssignemnt) {
   auto* heap = GetHeap();
-  NullStateAssignemnt<Persistent>(heap);
-  NullStateAssignemnt<WeakPersistent>(heap);
+  NullStateAssignment<Persistent>(heap);
+  NullStateAssignment<WeakPersistent>(heap);
+  NullStateAssignment<subtle::CrossThreadPersistent>(heap);
+  NullStateAssignment<subtle::WeakCrossThreadPersistent>(heap);
 }
 
 template <template <typename> class PersistentType>
@@ -322,6 +371,8 @@ TEST_F(PersistentTest, RawAssignment) {
   auto* heap = GetHeap();
   RawAssignment<Persistent>(heap);
   RawAssignment<WeakPersistent>(heap);
+  RawAssignment<subtle::CrossThreadPersistent>(heap);
+  RawAssignment<subtle::WeakCrossThreadPersistent>(heap);
 }
 
 template <template <typename> class PersistentType>
@@ -384,6 +435,8 @@ TEST_F(PersistentTest, CopyAssignment) {
   auto* heap = GetHeap();
   CopyAssignment<Persistent>(heap);
   CopyAssignment<WeakPersistent>(heap);
+  CopyAssignment<subtle::CrossThreadPersistent>(heap);
+  CopyAssignment<subtle::WeakCrossThreadPersistent>(heap);
 }
 
 template <template <typename> class PersistentType>
@@ -444,6 +497,8 @@ TEST_F(PersistentTest, MoveAssignment) {
   auto* heap = GetHeap();
   MoveAssignment<Persistent>(heap);
   MoveAssignment<WeakPersistent>(heap);
+  MoveAssignment<subtle::CrossThreadPersistent>(heap);
+  MoveAssignment<subtle::WeakCrossThreadPersistent>(heap);
 }
 
 template <template <typename> class PersistentType,
@@ -470,6 +525,12 @@ TEST_F(PersistentTest, MemberAssignment) {
   MemberAssignment<WeakPersistent, Member>(heap);
   MemberAssignment<WeakPersistent, WeakMember>(heap);
   MemberAssignment<WeakPersistent, UntracedMember>(heap);
+  MemberAssignment<subtle::CrossThreadPersistent, Member>(heap);
+  MemberAssignment<subtle::CrossThreadPersistent, WeakMember>(heap);
+  MemberAssignment<subtle::CrossThreadPersistent, UntracedMember>(heap);
+  MemberAssignment<subtle::WeakCrossThreadPersistent, Member>(heap);
+  MemberAssignment<subtle::WeakCrossThreadPersistent, WeakMember>(heap);
+  MemberAssignment<subtle::WeakCrossThreadPersistent, UntracedMember>(heap);
 }
 
 template <template <typename> class PersistentType>
@@ -488,6 +549,8 @@ TEST_F(PersistentTest, Clear) {
   auto* heap = GetHeap();
   ClearTest<Persistent>(heap);
   ClearTest<WeakPersistent>(heap);
+  ClearTest<subtle::CrossThreadPersistent>(heap);
+  ClearTest<subtle::WeakCrossThreadPersistent>(heap);
 }
 
 template <template <typename> class PersistentType>
@@ -507,6 +570,8 @@ TEST_F(PersistentTest, Release) {
   auto* heap = GetHeap();
   ReleaseTest<Persistent>(heap);
   ReleaseTest<WeakPersistent>(heap);
+  ReleaseTest<subtle::CrossThreadPersistent>(heap);
+  ReleaseTest<subtle::WeakCrossThreadPersistent>(heap);
 }
 
 template <template <typename> class PersistentType1,
@@ -630,6 +695,10 @@ TEST_F(PersistentTest, ClearOnHeapDestruction) {
   weak_persistent = MakeGarbageCollected<GCed>(heap->GetAllocationHandle());
   const Persistent<GCed> persistent_sentinel(kSentinelPointer);
   const WeakPersistent<GCed> weak_persistent_sentinel(kSentinelPointer);
+  const subtle::CrossThreadPersistent<GCed> cross_thread_persistent_sentinel(
+      kSentinelPointer);
+  const subtle::WeakCrossThreadPersistent<GCed>
+      cross_thread_weak_persistent_sentinel(kSentinelPointer);
   heap.reset();
 
   EXPECT_EQ(nullptr, persistent);
@@ -650,6 +719,14 @@ TEST_F(PersistentTest, LocalizedPersistent) {
     EXPECT_STREQ(expected_loc.FileName(), actual_loc.FileName());
     EXPECT_EQ(expected_loc.Line() + 1, actual_loc.Line());
   }
+  {
+    const auto expected_loc = SourceLocation::Current();
+    LocalizedCrossThreadPersistent<GCed> p = gced;
+    const auto actual_loc = p.Location();
+    EXPECT_STREQ(expected_loc.Function(), actual_loc.Function());
+    EXPECT_STREQ(expected_loc.FileName(), actual_loc.FileName());
+    EXPECT_EQ(expected_loc.Line() + 1, actual_loc.Line());
+  }
   {
     // Copy ctor doesn't copy source location.
     LocalizedPersistent<GCed> p1 = gced;
@@ -658,6 +735,14 @@ TEST_F(PersistentTest, LocalizedPersistent) {
     EXPECT_STREQ(p1.Location().FileName(), p2.Location().FileName());
     EXPECT_EQ(p1.Location().Line() + 1, p2.Location().Line());
   }
+  {
+    // Copy ctor doesn't copy source location.
+    LocalizedCrossThreadPersistent<GCed> p1 = gced;
+    LocalizedCrossThreadPersistent<GCed> p2 = p1;
+    EXPECT_STREQ(p1.Location().Function(), p2.Location().Function());
+    EXPECT_STREQ(p1.Location().FileName(), p2.Location().FileName());
+    EXPECT_EQ(p1.Location().Line() + 1, p2.Location().Line());
+  }
   {
     // Copy assignment doesn't copy source location.
     LocalizedPersistent<GCed> p1 = gced;
@@ -667,6 +752,15 @@ TEST_F(PersistentTest, LocalizedPersistent) {
     EXPECT_STREQ(p1.Location().FileName(), p2.Location().FileName());
     EXPECT_EQ(p1.Location().Line() + 1, p2.Location().Line());
   }
+  {
+    // Copy assignment doesn't copy source location.
+    LocalizedCrossThreadPersistent<GCed> p1 = gced;
+    LocalizedCrossThreadPersistent<GCed> p2;
+    p2 = p1;
+    EXPECT_STREQ(p1.Location().Function(), p2.Location().Function());
+    EXPECT_STREQ(p1.Location().FileName(), p2.Location().FileName());
+    EXPECT_EQ(p1.Location().Line() + 1, p2.Location().Line());
+  }
   {
     // Clearing doesn't clear source location.
     LocalizedPersistent<GCed> p1 = gced;
@@ -676,6 +770,15 @@ TEST_F(PersistentTest, LocalizedPersistent) {
     EXPECT_STREQ(p1.Location().FileName(), p2.Location().FileName());
     EXPECT_EQ(p1.Location().Line() + 1, p2.Location().Line());
   }
+  {
+    // Clearing doesn't clear source location.
+    LocalizedCrossThreadPersistent<GCed> p1 = gced;
+    LocalizedCrossThreadPersistent<GCed> p2 = gced;
+    p2.Clear();
+    EXPECT_STREQ(p1.Location().Function(), p2.Location().Function());
+    EXPECT_STREQ(p1.Location().FileName(), p2.Location().FileName());
+    EXPECT_EQ(p1.Location().Line() + 1, p2.Location().Line());
+  }
   {
     LocalizedPersistent<GCed> p1 = gced;
     const auto expected_loc = p1.Location();
@@ -684,6 +787,14 @@ TEST_F(PersistentTest, LocalizedPersistent) {
     EXPECT_STREQ(expected_loc.FileName(), p2.Location().FileName());
     EXPECT_EQ(expected_loc.Line(), p2.Location().Line());
   }
+  {
+    LocalizedCrossThreadPersistent<GCed> p1 = gced;
+    const auto expected_loc = p1.Location();
+    LocalizedCrossThreadPersistent<GCed> p2 = std::move(p1);
+    EXPECT_STREQ(expected_loc.Function(), p2.Location().Function());
+    EXPECT_STREQ(expected_loc.FileName(), p2.Location().FileName());
+    EXPECT_EQ(expected_loc.Line(), p2.Location().Line());
+  }
   {
     LocalizedPersistent<GCed> p1 = gced;
     const auto expected_loc = p1.Location();
@@ -693,6 +804,15 @@ TEST_F(PersistentTest, LocalizedPersistent) {
     EXPECT_STREQ(expected_loc.FileName(), p2.Location().FileName());
     EXPECT_EQ(expected_loc.Line(), p2.Location().Line());
   }
+  {
+    LocalizedCrossThreadPersistent<GCed> p1 = gced;
+    const auto expected_loc = p1.Location();
+    LocalizedCrossThreadPersistent<GCed> p2;
+    p2 = std::move(p1);
+    EXPECT_STREQ(expected_loc.Function(), p2.Location().Function());
+    EXPECT_STREQ(expected_loc.FileName(), p2.Location().FileName());
+    EXPECT_EQ(expected_loc.Line(), p2.Location().Line());
+  }
 }
 #endif