[oilpan]: Make thread shutdown more robust.

Source/platform/heap/ThreadState.cpp

Index: Source/platform/heap/ThreadState.cpp
diff --git a/Source/platform/heap/ThreadState.cpp b/Source/platform/heap/ThreadState.cpp
index b15a2362232652299e2b378a40b89fa7bff432c5..56bb4dc8402f1454ec7dc179fecad7a111e5baa9 100644
--- a/Source/platform/heap/ThreadState.cpp
+++ b/Source/platform/heap/ThreadState.cpp
@@ -102,6 +102,19 @@ static Mutex& threadAttachMutex()
     return mutex;
 }
 
+// The threadShutdownMutex is used to synchronize thread shutdown
+// since the thread local GC, as of now, cannot run in parallel
+// with other thread local GCs since it is using the global marking
+// stack. It can also not run in parallel with a global GC, but
+// that is honored by not entering a safepoint while doing the
+// thread local GC, meaning a request for a global GC would time
+// out.
+static Mutex& threadShutdownMutex()
+{
+    AtomicallyInitializedStatic(Mutex&, mutex = *new Mutex);
+    return mutex;
+}
+
 static double lockingTimeout()
 {
     // Wait time for parking all threads is at most 100 MS.
@@ -260,6 +273,16 @@ private:
     ThreadCondition m_resume;
 };
 
+
+BaseHeapPage::BaseHeapPage(PageMemory* storage, const GCInfo* gcInfo, ThreadState* state)
+    : m_storage(storage)
+    , m_gcInfo(gcInfo)
+    , m_threadState(state)
+    , m_padding(0)
+{
+    ASSERT(isPageHeaderAddress(reinterpret_cast<Address>(this)));
+}
+
 ThreadState::ThreadState()
     : m_thread(currentThread())
     , m_persistents(adoptPtr(new PersistentAnchor()))
@@ -287,9 +310,9 @@ ThreadState::ThreadState()
     m_statsAfterLastGC.clear();
     // First allocate the general heap, second iterate through to
     // allocate the type specific heaps
-    m_heaps[GeneralHeap] = new ThreadHeap<FinalizedHeapObjectHeader>(this);
+    m_heaps[GeneralHeap] = new ThreadHeap<FinalizedHeapObjectHeader>(this, GeneralHeap);
     for (int i = GeneralHeap + 1; i < NumberOfHeaps; i++)
-        m_heaps[i] = new ThreadHeap<HeapObjectHeader>(this);
+        m_heaps[i] = new ThreadHeap<HeapObjectHeader>(this, i);
 
     CallbackStack::init(&m_weakCallbackStack);
 }
@@ -375,14 +398,29 @@ void ThreadState::cleanup()
     for (size_t i = 0; i < m_cleanupTasks.size(); i++)
         m_cleanupTasks[i]->preCleanup();
 
-    // After this GC we expect heap to be empty because
-    // preCleanup tasks should have cleared all persistent
-    // handles that were externally owned.
-    Heap::collectAllGarbage();
+    {
+        // We enter a safepoint while waiting for the thread shutdown mutex.
+        SafePointAwareMutexLocker locker(threadShutdownMutex());
+
+        // Set a flag on all this thread's heap pages to ensure we don't trace
+        // outside this thread's heap pages.
+        setupHeapsForShutdown();
+
+        // Do thread local GC's as long as the count of thread local Persistents
+        // changes and is above zero.
+        PersistentAnchor* anchor = static_cast<PersistentAnchor*>(m_persistents.get());
+        int oldCount = 0;
+        int curCount = anchor->numberOfPersistents();
+        while (curCount > 0 && curCount != oldCount) {
+            Heap::collectGarbageForThread(this, false);
+            oldCount = curCount;
+            curCount = anchor->numberOfPersistents();
+        }
 
-    // Verify that all heaps are empty now.
-    for (int i = 0; i < NumberOfHeaps; i++)
-        m_heaps[i]->assertEmpty();
+        // Do a final sweep to finalize any objects pointed to by persistents
+        // collected in the last round of GC above.
+        Heap::collectGarbageForThread(this, true);
+    }
 
     for (size_t i = 0; i < m_cleanupTasks.size(); i++)
         m_cleanupTasks[i]->postCleanup();
@@ -407,6 +445,10 @@ void ThreadState::detach()
         state->leaveSafePoint();
         ASSERT(attachedThreads().contains(state));
         attachedThreads().remove(state);
+        // Deleting the thread state also destroys the thread's heaps at which
+        // point all the thread's pages are added to the orphanedPagePool.
+        // Subsequently they are promoted to the memoryPool once they are no

[zerny-chromium, 2014/07/07 12:11:56]

s/promoted/moved

[wibling-chromium, 2014/07/07 13:50:07]

Done.

+        // longer traced by a global GC.
         delete state;
     }
     shutdownHeapIfNecessary();
@@ -427,6 +469,17 @@ void ThreadState::visitRoots(Visitor* visitor)
         (*it)->trace(visitor);
 }
 
+void ThreadState::visitLocalRoots(Visitor* visitor)
+{
+    // We assume there are no CrossThreadPersistents pointing to this thread when
+    // it is being destructed since it would typically have been in a refcounted
+    // object which should have been destructed prior to shutting down the thread.
+    // If there are any we treat it similar to when we find a dead object with
+    // a member pointing to this thread's heap and allow it being marked, but
+    // do nothing when popping the trace method.

[zerny-chromium, 2014/07/07 12:11:56]

Simpler:

We assume there are no CrossThreadPersistents to this threads heap. If there is,
it will be handled in the same way as any other cross-thread pointer pointing to
an orphaned page.

[wibling-chromium, 2014/07/07 13:50:07]

Done. Much better:)

+    m_persistents->trace(visitor);
+}
+
 NO_SANITIZE_ADDRESS
 void ThreadState::visitAsanFakeStackForPointer(Visitor* visitor, Address ptr)
 {
@@ -549,7 +602,7 @@ void ThreadState::pushWeakObjectPointerCallback(void* object, WeakPointerCallbac
 
 bool ThreadState::popAndInvokeWeakPointerCallback(Visitor* visitor)
 {
-    return m_weakCallbackStack->popAndInvokeCallback(&m_weakCallbackStack, visitor);
+    return m_weakCallbackStack->popAndInvokeCallback<false>(&m_weakCallbackStack, visitor);

[zerny-chromium, 2014/07/07 12:11:56]

Nit: maybe we make an type/enum for ThreadLocal/GlobalGC

[wibling-chromium, 2014/07/07 13:50:07]

I prefer keeping it as a bool to keep the if in the invokeCallback methods
simpler, ie. no ==.

 }
 
 PersistentNode* ThreadState::globalRoots()
@@ -687,15 +740,28 @@ void ThreadState::prepareForGC()
     for (int i = 0; i < NumberOfHeaps; i++) {
         BaseHeap* heap = m_heaps[i];
         heap->makeConsistentForGC();
-        // If there are parked threads with outstanding sweep requests, clear their mark bits.
-        // This happens if a thread did not have time to wake up and sweep,
-        // before the next GC arrived.
-        if (sweepRequested())
-            heap->clearMarks();
+        // If there are parked threads with outstanding sweep requests, clear their mark bits,
+        // and mark any of their dead objects as dead. The latter is used to ensure the next
+        // GC marking does not revive already dead objects. If we revived a dead object we
+        // could end up tracing into garbage or the middle of another object via the revived
+        // object.
+        // The case of a thread missing a sweep happens if it did not have time to wake up
+        // and sweep, before the next GC arrived.
+        if (sweepRequested()) {
+            heap->clearLiveAndMarkDead();
+        }

[zerny-chromium, 2014/07/07 12:11:56]

Nit: no curlies 

[wibling-chromium, 2014/07/07 13:50:07]

Done.

     }
     setSweepRequested();
 }
 
+void ThreadState::setupHeapsForShutdown()
+{
+    for (int i = 0; i < NumberOfHeaps; i++) {
+        BaseHeap* heap = m_heaps[i];
+        heap->setShutdown();
+    }
+}
+
 BaseHeapPage* ThreadState::heapPageFromAddress(Address address)
 {
     BaseHeapPage* cachedPage = heapContainsCache()->lookup(address);