[oilpan]: Make thread shutdown more robust.

Source/platform/heap/ThreadState.cpp

 /*
  * Copyright (C) 2013 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 84 matching lines @@
 uint8_t ThreadState::s_mainThreadStateStorage[sizeof(ThreadState)];
 SafePointBarrier* ThreadState::s_safePointBarrier = 0;
 bool ThreadState::s_inGC = false;
 
 static Mutex& threadAttachMutex()
 {
     AtomicallyInitializedStatic(Mutex&, mutex = *new Mutex);
     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.
     return 0.100;
 }
 
 
 typedef void (*PushAllRegistersCallback)(SafePointBarrier*, ThreadState*, intptr_t*);
 extern "C" void pushAllRegisters(SafePointBarrier*, ThreadState*, PushAllRegistersCallback);
 
@@ skipping 138 matching lines @@
         barrier->doEnterSafePoint(state, stackEnd);
     }
 
     volatile int m_canResume;
     volatile int m_unparkedThreadCount;
     Mutex m_mutex;
     ThreadCondition m_parked;
     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()))
     , m_startOfStack(reinterpret_cast<intptr_t*>(getStackStart()))
     , m_endOfStack(reinterpret_cast<intptr_t*>(getStackStart()))
     , m_safePointScopeMarker(0)
     , m_atSafePoint(false)
     , m_interruptors()
     , m_gcRequested(false)
     , m_forcePreciseGCForTesting(false)
     , m_sweepRequested(0)
     , m_sweepInProgress(false)
     , m_noAllocationCount(0)
     , m_inGC(false)
     , m_heapContainsCache(adoptPtr(new HeapContainsCache()))
     , m_isCleaningUp(false)
 #if defined(ADDRESS_SANITIZER)
     , m_asanFakeStack(__asan_get_current_fake_stack())
 #endif
 {
     ASSERT(!**s_threadSpecific);
     **s_threadSpecific = this;
 
     m_stats.clear();
     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);
 }
 
 ThreadState::~ThreadState()
 {
     checkThread();
     CallbackStack::shutdown(&m_weakCallbackStack);
     for (int i = GeneralHeap; i < NumberOfHeaps; i++)
         delete m_heaps[i];
@@ skipping 65 matching lines @@
 
 void ThreadState::cleanup()
 {
     // From here on ignore all conservatively discovered
     // pointers into the heap owned by this thread.
     m_isCleaningUp = true;
 
     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());
 
-    // Verify that all heaps are empty now.
-    for (int i = 0; i < NumberOfHeaps; i++)
-        m_heaps[i]->assertEmpty();
+        // 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();
+        }
+
+        // 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();
 
     m_cleanupTasks.clear();
 }
 
 void ThreadState::detach()
 {
     ThreadState* state = current();
     state->cleanup();
 
     // Enter a safe point before trying to acquire threadAttachMutex
     // to avoid dead lock if another thread is preparing for GC, has acquired
     // threadAttachMutex and waiting for other threads to pause or reach a
     // safepoint.
     if (!state->isAtSafePoint())
         state->enterSafePointWithoutPointers();
 
     {
         MutexLocker locker(threadAttachMutex());
         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();
 }
 
 void ThreadState::visitRoots(Visitor* visitor)
 {
     {
         // All threads are at safepoints so this is not strictly necessary.
         // However we acquire the mutex to make mutation and traversal of this
         // list symmetrical.
         MutexLocker locker(globalRootsMutex());
         globalRoots()->trace(visitor);
     }
 
     AttachedThreadStateSet& threads = attachedThreads();
     for (AttachedThreadStateSet::iterator it = threads.begin(), end = threads.end(); it != end; ++it)
         (*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)
 {
 #if defined(ADDRESS_SANITIZER)
     Address* start = reinterpret_cast<Address*>(m_startOfStack);
     Address* end = reinterpret_cast<Address*>(m_endOfStack);
     Address* fakeFrameStart = 0;
     Address* fakeFrameEnd = 0;
     Address* maybeFakeFrame = reinterpret_cast<Address*>(ptr);
     Address* realFrameForFakeFrame =
@@ skipping 102 matching lines @@
 #endif
 
 void ThreadState::pushWeakObjectPointerCallback(void* object, WeakPointerCallback callback)
 {
     CallbackStack::Item* slot = m_weakCallbackStack->allocateEntry(&m_weakCallbackStack);
     *slot = CallbackStack::Item(object, callback);
 }
 
 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()
 {
     AtomicallyInitializedStatic(PersistentNode*, anchor = new PersistentAnchor);
     return anchor;
 }
 
 Mutex& ThreadState::globalRootsMutex()
 {
@@ skipping 117 matching lines @@
 {
     for (int i = 0; i < NumberOfHeaps; i++)
         m_heaps[i]->makeConsistentForGC();
 }
 
 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);
 #ifdef NDEBUG
     if (cachedPage)
         return cachedPage;
 #endif
 
     for (int i = 0; i < NumberOfHeaps; i++) {
         BaseHeapPage* page = m_heaps[i]->heapPageFromAddress(address);
@@ skipping 206 matching lines @@
                 threadAttachMutex().unlock();
             return gcInfo;
         }
     }
     if (needLockForIteration)
         threadAttachMutex().unlock();
     return 0;
 }
 #endif
 }