Oilpan: Clear all Persistents on thread termination

Clear all persistents on thread termination, because when we have a bug and it leaves a Persistent behind, it will not be a stale pointer and just causes null dereference.

Since we disabled PersistentHeapCollections on non-main threads we can assume all PersistentNode::Self() can be cast to Persistent<DummyGCBase>

Bug: 831117
Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel
Change-Id: Ib55b280f8e7c5d966ce43d3f60f2af873cd50739
Reviewed-on: https://chromium-review.googlesource.com/1025547
Commit-Queue: Keishi Hattori <keishi@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561418}

third_party/blink/renderer/platform/heap/persistent_node.cc

@@ -126,6 +126,29 @@ void PersistentRegion::TracePersistentNodes(Visitor* visitor,
 #endif
 }
 
+void PersistentRegion::PrepareForThreadStateTermination() {
+  DCHECK(!IsMainThread());
+  PersistentNodeSlots* slots = slots_;
+  while (slots) {
+    for (int i = 0; i < PersistentNodeSlots::kSlotCount; ++i) {
+      PersistentNode* node = &slots->slot_[i];
+      if (node->IsUnused())
+        continue;
+      // It is safe to cast to Persistent<DummyGCBase> because persistent heap
+      // collections are banned in non-main threads.
+      Persistent<DummyGCBase>* persistent =
+          reinterpret_cast<Persistent<DummyGCBase>*>(node->Self());
+      DCHECK(persistent);
+      persistent->Clear();
+      DCHECK(node->IsUnused());
+    }
+    slots = slots->next_;
+  }
+#if DCHECK_IS_ON()
+  DCHECK_EQ(persistent_count_, 0);
+#endif
+}
+
 bool CrossThreadPersistentRegion::ShouldTracePersistentNode(
     Visitor* visitor,
     PersistentNode* node) {
@@ -146,9 +169,6 @@ void CrossThreadPersistentRegion::PrepareForThreadStateTermination(
   // out the underlying heap reference.
   RecursiveMutexLocker lock(ProcessHeap::CrossThreadPersistentMutex());
 
-  // TODO(sof): consider ways of reducing overhead. (e.g., tracking number of
-  // active CrossThreadPersistent<>s pointing into the heaps of each ThreadState
-  // and use that count to bail out early.)
   PersistentNodeSlots* slots = persistent_region_->slots_;
   while (slots) {
     for (int i = 0; i < PersistentNodeSlots::kSlotCount; ++i) {

third_party/blink/renderer/platform/heap/persistent_node.h

@@ -151,6 +151,7 @@ class PLATFORM_EXPORT PersistentRegion final {
       Visitor*,
       ShouldTraceCallback = PersistentRegion::ShouldTracePersistentNode);
   int NumberOfPersistents();
+  void PrepareForThreadStateTermination();
 
  private:
   friend CrossThreadPersistentRegion;

third_party/blink/renderer/platform/heap/thread_state.cc

@@ -89,6 +89,8 @@ const size_t kDefaultAllocatedObjectSizeThreshold = 100 * 1024;
 // doesn't cause jank even though it is scheduled as a normal task.
 const double kIncrementalMarkingStepDurationInSeconds = 0.001;
 
+constexpr size_t kMaxTerminationGCLoops = 20;
+
 namespace {
 
 const char* GcReasonString(BlinkGC::GCReason reason) {
@@ -236,9 +238,23 @@ void ThreadState::RunTerminationGC() {
     old_count = current_count;
     current_count = GetPersistentRegion()->NumberOfPersistents();
   }
+
   // We should not have any persistents left when getting to this point,
-  // if we have it is probably a bug so adding a debug ASSERT to catch this.
-  DCHECK(!current_count);
+  // if we have it is a bug, and we have a reference cycle or a missing
+  // RegisterAsStaticReference. Clearing out all the Persistents will avoid
+  // stale pointers and gets them reported as nullptr dereferences.
+  if (!current_count) {
+    for (size_t i = 0; i < kMaxTerminationGCLoops &&
+                       GetPersistentRegion()->NumberOfPersistents();
+         i++) {
+      GetPersistentRegion()->PrepareForThreadStateTermination();
+      CollectGarbage(BlinkGC::kNoHeapPointersOnStack, BlinkGC::kAtomicMarking,
+                     BlinkGC::kEagerSweeping, BlinkGC::kThreadTerminationGC);
+    }
+  }
+
+  CHECK(!GetPersistentRegion()->NumberOfPersistents());
+
   // All of pre-finalizers should be consumed.
   DCHECK(ordered_pre_finalizers_.IsEmpty());
   CHECK_EQ(GcState(), kNoGCScheduled);