Reland "cppgc: Add targeted CHECK for diagnosing Peristent issue"

This is a reland of 4997ce58

Original change's description:
> cppgc: Add targeted CHECK for diagnosing Peristent issue
>
> The added CHECK aims at finding problems where Peristent is used off
> the owning thread.
>
> Bug: chromium:1253650, chromium:1243257
> Change-Id: Ia0cbc6005aba38c0d98197ed18c3b40dd2dc33fd
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3306972
> Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
> Reviewed-by: Anton Bikineev <bikineev@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#78137}

Bug: chromium:1253650, chromium:1243257
Change-Id: I9ef72d3f649c1a4504417c4cd7728e5000675405
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3306487Reviewed-by: Anton Bikineev <bikineev@chromium.org>
Commit-Queue: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#78146}

include/cppgc/internal/persistent-node.h

@@ -141,18 +141,18 @@ class V8_EXPORT PersistentRegion final : public PersistentRegionBase {
   PersistentRegion& operator=(const PersistentRegion&) = delete;
 
   V8_INLINE PersistentNode* AllocateNode(void* owner, TraceCallback trace) {
-    CPPGC_DCHECK(IsCreationThread());
+    CPPGC_CHECK(IsCreationThread());
     return PersistentRegionBase::AllocateNode(owner, trace);
   }
 
   V8_INLINE void FreeNode(PersistentNode* node) {
-    CPPGC_DCHECK(IsCreationThread());
+    CPPGC_CHECK(IsCreationThread());
     PersistentRegionBase::FreeNode(node);
   }
 
- private:
   bool IsCreationThread();
 
+ private:
   int creation_thread_id_;
 };
 

include/cppgc/persistent.h

@@ -190,7 +190,13 @@ class BasicPersistent final : public PersistentBase,
   // based on their actual types.
   V8_CLANG_NO_SANITIZE("cfi-unrelated-cast") T* Get() const {
     // TODO(chromium:1253650): Temporary CHECK to diagnose issues.
-    CPPGC_CHECK(IsValid() == !!GetNode());
+    if (IsValid()) {
+      CPPGC_CHECK(
+          WeaknessPolicy::GetPersistentRegion(GetValue()).IsCreationThread());
+      CPPGC_CHECK(GetNode() != nullptr);
+    } else {
+      CPPGC_CHECK(GetNode() == nullptr);
+    }
 
     // The const_cast below removes the constness from PersistentBase storage.
     // The following static_cast re-adds any constness if specified through the
@@ -200,7 +206,13 @@ class BasicPersistent final : public PersistentBase,
 
   void Clear() {
     // TODO(chromium:1253650): Temporary CHECK to diagnose issues.
-    CPPGC_CHECK(IsValid() == !!GetNode());
+    if (IsValid()) {
+      CPPGC_CHECK(
+          WeaknessPolicy::GetPersistentRegion(GetValue()).IsCreationThread());
+      CPPGC_CHECK(GetNode() != nullptr);
+    } else {
+      CPPGC_CHECK(GetNode() == nullptr);
+    }
     // Simplified version of `Assign()` to allow calling without a complete type
     // `T`.
     if (IsValid()) {

src/heap/cppgc/persistent-node.cc

@@ -103,6 +103,19 @@ void PersistentRegionBase::Trace(Visitor* visitor) {
                nodes_.end());
 }
 
+namespace {
+
+thread_local int thread_id = 0;
+
+int GetCurrentThreadId() {
+  if (thread_id == 0) {
+    thread_id = v8::base::OS::GetCurrentThreadId();
+  }
+  return thread_id;
+}
+
+}  // namespace
+
 PersistentRegion::PersistentRegion(const FatalOutOfMemoryHandler& oom_handler)
     : PersistentRegionBase(oom_handler),
       creation_thread_id_(v8::base::OS::GetCurrentThreadId()) {
@@ -110,7 +123,10 @@ PersistentRegion::PersistentRegion(const FatalOutOfMemoryHandler& oom_handler)
 }
 
 bool PersistentRegion::IsCreationThread() {
-  return creation_thread_id_ == v8::base::OS::GetCurrentThreadId();
+  // Short circuit using TLS cache. If that doesn't work (e.g. in TLS teardown)
+  // fall back to the more expensive call in base.
+  return creation_thread_id_ == GetCurrentThreadId() ||
+         creation_thread_id_ == v8::base::OS::GetCurrentThreadId();
 }
 
 PersistentRegionLock::PersistentRegionLock() {