Unify and generalize thread static persistent finalization.

third_party/WebKit/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 202 matching lines @@
     }
     ASSERT(checkThread());
 
     // Finish sweeping.
     completeSweep();
 
     // From here on ignore all conservatively discovered
     // pointers into the heap owned by this thread.
     m_isTerminating = true;
 
-    ThreadState::callThreadShutdownHooks();
+    releaseStaticPersistentNodes();
 
     // Set the terminate flag on all heap pages of this thread. This is used to
     // ensure we don't trace pages on other threads that are not part of the
     // thread local GC.
     prepareForThreadStateTermination();
 
     ProcessHeap::crossThreadPersistentRegion().prepareForThreadStateTermination(this);
 
     // Do thread local GC's as long as the count of thread local Persistents
     // changes and is above zero.
@@ skipping 14 matching lines @@
 
     // Add pages to the orphaned page pool to ensure any global GCs from this point
     // on will not trace objects on this thread's arenas.
     cleanupPages();
 }
 
 void ThreadState::cleanupMainThread()
 {
     ASSERT(isMainThread());
 
+    releaseStaticPersistentNodes();
+
 #if defined(LEAK_SANITIZER)
-    // If LSan is about to perform leak detection, release all the registered
-    // static Persistent<> root references to global caches that Blink keeps,
-    // followed by GCs to clear out all they referred to.
+    // If LSan is about to perform leak detection, after having released all
+    // the registered static Persistent<> root references to global caches
+    // that Blink keeps, follow up with a round of GCs to clear out all
+    // what they referred to.
     //
     // This is not needed for caches over non-Oilpan objects, as they're
     // not scanned by LSan due to being held in non-global storage
     // ("static" references inside functions/methods.)
-    releaseStaticPersistentNodes();
     ThreadHeap::collectAllGarbage();
 #endif
 
     // Finish sweeping before shutting down V8. Otherwise, some destructor
     // may access V8 and cause crashes.
     completeSweep();
 
     // It is unsafe to trigger GCs after this point because some
     // destructor may access already-detached V8 and cause crashes.
     // Also it is useless. So we forbid GCs.
     enterGCForbiddenScope();
 }
 
 void ThreadState::detachMainThread()
 {
     // 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.
     ThreadState* state = mainThreadState();
     ASSERT(!state->isSweepingInProgress());
 
     state->heap().detach(state);
     state->~ThreadState();
 }
 
-void ThreadState::callThreadShutdownHooks()
-{
-    // Invoke the cleanup hooks. This gives an opportunity to release any
-    // persistent handles that may exist, e.g. in thread-specific static
-    // locals.
-    for (const OwnPtr<SameThreadClosure>& hook : m_threadShutdownHooks)
-        (*hook)();
-}
-
 void ThreadState::detachCurrentThread()
 {
     ThreadState* state = current();
     state->heap().detach(state);
     RELEASE_ASSERT(state->gcState() == ThreadState::NoGCScheduled);
     delete state;
 }
 
 NO_SANITIZE_ADDRESS
 void ThreadState::visitAsanFakeStackForPointer(Visitor* visitor, Address ptr)
@@ skipping 997 matching lines @@
 void ThreadState::addInterruptor(PassOwnPtr<BlinkGCInterruptor> interruptor)
 {
     ASSERT(checkThread());
     SafePointScope scope(BlinkGC::HeapPointersOnStack);
     {
         MutexLocker locker(m_heap->threadAttachMutex());
         m_interruptors.append(interruptor);
     }
 }
 
-void ThreadState::registerThreadShutdownHook(PassOwnPtr<SameThreadClosure> hook)
+void ThreadState::registerStaticPersistentNode(PersistentNode* node, PersistentClearCallback callback)
 {
-    ASSERT(checkThread());
-    ASSERT(!isTerminating());
-    m_threadShutdownHooks.append(hook);
-}
-
 #if defined(LEAK_SANITIZER)
-void ThreadState::registerStaticPersistentNode(PersistentNode* node)
-{
     if (m_disabledStaticPersistentsRegistration)
         return;
+#endif
 
     ASSERT(!m_staticPersistents.contains(node));
-    m_staticPersistents.add(node);
+    m_staticPersistents.add(node, callback);
 }
 
 void ThreadState::releaseStaticPersistentNodes()
 {
-    for (PersistentNode* node : m_staticPersistents)
-        getPersistentRegion()->freePersistentNode(node);
+    HashMap<PersistentNode*, ThreadState::PersistentClearCallback> staticPersistents;
+    staticPersistents.swap(m_staticPersistents);
 
-    m_staticPersistents.clear();
+    PersistentRegion* persistentRegion = getPersistentRegion();
+    for (const auto& it : staticPersistents)
+        persistentRegion->releasePersistentNode(it.key, it.value);
 }
 
+void ThreadState::freePersistentNode(PersistentNode* persistentNode)
+{
+    PersistentRegion* persistentRegion = getPersistentRegion();
+    persistentRegion->freePersistentNode(persistentNode);
+    // Do not allow static persistents to be freed before
+    // they're all released in releaseStaticPersistentNodes().
+    //
+    // There's no fundamental reason why this couldn't be supported,
+    // but no known use for it.

[haraken, 2016/04/25 08:39:23]

Yeah, I prefer adding this restriction too. registerStaticPersistentNode should
be used for only "static" persistent nodes (i.e., never freed).

[sof, 2016/04/25 08:53:05]

Agreed; a thread needing to free a persistent while running would be it, but I'd
like to see where & when that is needed first. 

+    ASSERT(!m_staticPersistents.contains(persistentNode));
+}
+
+#if defined(LEAK_SANITIZER)
 void ThreadState::enterStaticReferenceRegistrationDisabledScope()
 {
     m_disabledStaticPersistentsRegistration++;
 }
 
 void ThreadState::leaveStaticReferenceRegistrationDisabledScope()
 {
     ASSERT(m_disabledStaticPersistentsRegistration);
     m_disabledStaticPersistentsRegistration--;
 }
@@ skipping 155 matching lines @@
     threadDump->addScalar("dead_count", "objects", totalDeadCount);
     threadDump->addScalar("live_size", "bytes", totalLiveSize);
     threadDump->addScalar("dead_size", "bytes", totalDeadSize);
 
     WebMemoryAllocatorDump* heapsDump = BlinkGCMemoryDumpProvider::instance()->createMemoryAllocatorDumpForCurrentGC(heapsDumpName);
     WebMemoryAllocatorDump* classesDump = BlinkGCMemoryDumpProvider::instance()->createMemoryAllocatorDumpForCurrentGC(classesDumpName);
     BlinkGCMemoryDumpProvider::instance()->currentProcessMemoryDump()->addOwnershipEdge(classesDump->guid(), heapsDump->guid());
 }
 
 } // namespace blink