Invalidate cross-thread persistents on heap termination.

Source/platform/heap/PersistentNode.cpp

Index: Source/platform/heap/PersistentNode.cpp
diff --git a/Source/platform/heap/PersistentNode.cpp b/Source/platform/heap/PersistentNode.cpp
index d8e22fc6cff30e688c7ac5ba96c984c4e041e02c..7d145a1ce7adbbfeb87c4327bd0e5a4cf27e3a68 100644
--- a/Source/platform/heap/PersistentNode.cpp
+++ b/Source/platform/heap/PersistentNode.cpp
@@ -5,6 +5,8 @@
 #include "config.h"
 #include "platform/heap/PersistentNode.h"
 
+#include "platform/heap/Handle.h"
+
 namespace blink {
 
 PersistentRegion::~PersistentRegion()
@@ -60,6 +62,29 @@ void PersistentRegion::tracePersistentNodes(Visitor* visitor)
         int freeCount = 0;
         for (int i = 0; i < PersistentNodeSlots::slotCount; ++i) {
             PersistentNode* node = &slots->m_slot[i];
+            if (!node->isValid()) {
+                // We do allow CrossThreadPersistent<> handles to exist beyond
+                // the lifetime of the object they refer to and keep alive.
+                // This to allow an Oilpan thread to shut down and terminate

[haraken, 2015/08/03 00:45:25]

This is to allow

+                // without first coordinating with any and all threads that hold
+                // CrossThreadPersistent<>s pointing into its heaps - to have
+                // them first release those references - before the thread
+                // proceeds to detach its heaps and shuts down.
+                //
+                // The cross-thread persistent variant eases the same-thread
+                // destruction restriction of a persistent (while also keeping
+                // the object it refers to alive). The persistent reference isn't
+                // however so strong so as to prevent a heap being detached and
+                // destructed. Just that the destruction of the cross-thread
+                // persistent may happen later and independently from the
+                // heap of the object it points to.
+                //
+                // It is clearly not safe to continue tracing cross-thread
+                // persistents referring to objects and heaps that have been
+                // destructed, so these will be marked as invalid and skipped here.
+                ++persistentCount;
+                continue;
+            }
             if (node->isUnused()) {
                 if (!freeListNext)
                     freeListLast = node;
@@ -90,4 +115,37 @@ void PersistentRegion::tracePersistentNodes(Visitor* visitor)
     ASSERT(persistentCount == m_persistentCount);
 }
 
+void CrossThreadPersistentRegion::prepareForThreadStateTermination(ThreadState* threadState)
+{
+    // For heaps belonging to a thread that's detaching, any cross-thread persistents
+    // pointing into them needs to be disabled.
+    MutexLocker lock(m_mutex);
+
+    class Object;
+    using GCObject = GarbageCollected<Object>;
+
+    // TODO(sof): consider ways of reducing the overhead of this invalidation.
+    // (e.g., tracking number of active CrossThreadPersistent<>s pointing
+    // into the heaps of each ThreadState & use that count to terminate traversal.)

[haraken, 2015/08/03 00:45:25]

Given that the number of CrossThreadPersistents should be limited, I'm not
worried about the overhead very much.

[sof, 2015/08/03 08:30:04]

I'd like to revisit and address, but thread local shutdowns aren't that
plentiful to boot.

+    PersistentNodeSlots* slots = m_persistentRegion->m_slots;
+    while (slots) {
+        for (int i = 0; i < PersistentNodeSlots::slotCount; ++i) {
+            if (!slots->m_slot[i].isValid() || slots->m_slot[i].isUnused())
+                continue;
+
+            // 'self' is in use, containing the cross-thread persistent wrapper object.
+            CrossThreadPersistent<GCObject>* persistent = reinterpret_cast<CrossThreadPersistent<GCObject>*>(slots->m_slot[i].self());
+            ASSERT(persistent);
+            void* rawObject = persistent->get();
+            if (!rawObject)
+                continue;
+            BasePage* page = pageFromObject(rawObject);
+            ASSERT(page);
+            if (page->heap()->threadState() == threadState)
+                slots->m_slot[i].invalidate();
+        }
+        slots = slots->m_next;
+    }
+}
+
 } // namespace blink