Oilpan: catch some self-referential leaks (main thread.)

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 58 matching lines @@
 #include <pthread_np.h>
 #endif
 
 namespace blink {
 
 WTF::ThreadSpecific<ThreadState*>* ThreadState::s_threadSpecific = nullptr;
 uintptr_t ThreadState::s_mainThreadStackStart = 0;
 uintptr_t ThreadState::s_mainThreadUnderestimatedStackSize = 0;
 uint8_t ThreadState::s_mainThreadStateStorage[sizeof(ThreadState)];
 SafePointBarrier* ThreadState::s_safePointBarrier = nullptr;
+#if ENABLE(ASSERT)
+int ThreadState::s_selfKeepAliveAllocationsOnMainThread = 0;
+#endif
 
 RecursiveMutex& ThreadState::threadAttachMutex()
 {
     AtomicallyInitializedStaticReference(RecursiveMutex, mutex, (new RecursiveMutex));
     return mutex;
 }
 
 ThreadState::ThreadState()
     : m_thread(currentThread())
     , m_persistentRegion(adoptPtr(new PersistentRegion()))
@@ skipping 70 matching lines @@
 
     // Thread-local storage shouldn't be disposed, so we don't call ~ThreadSpecific().
 }
 
 void ThreadState::attachMainThread()
 {
     RELEASE_ASSERT(!Heap::s_shutdownCalled);
     MutexLocker locker(threadAttachMutex());
     ThreadState* state = new(s_mainThreadStateStorage) ThreadState();
     attachedThreads().add(state);
+#if ENABLE(ASSERT)
+    s_selfKeepAliveAllocationsOnMainThread = 0;
+#endif
 }
 
 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();
 
     // 1. Finish sweeping.
     state->completeSweep();
     {
         SafePointAwareMutexLocker locker(threadAttachMutex(), NoHeapPointersOnStack);
 
         // 2. Add the main thread's heap pages to the orphaned pool.
         state->cleanupPages();
 
         // 3. Detach the main thread.
         ASSERT(attachedThreads().contains(state));
         attachedThreads().remove(state);
         state->~ThreadState();
     }
+    // Catch out any self-referential leaks created by the main thread.
+    ASSERT(s_selfKeepAliveAllocationsOnMainThread == 0);
     shutdownHeapIfNecessary();
 }
 
 void ThreadState::shutdownHeapIfNecessary()
 {
     // We don't need to enter a safe point before acquiring threadAttachMutex
     // because this thread is already detached.
 
     MutexLocker locker(threadAttachMutex());
     // We start shutting down the heap if there is no running thread
@@ skipping 1328 matching lines @@
     for (ClassAgeCountsMap::const_iterator it = classAgeCounts.begin(), end = classAgeCounts.end(); it != end; ++it) {
         json->beginArray(it->key.ascii().data());
         for (size_t age = 0; age <= maxHeapObjectAge; ++age)
             json->pushInteger(it->value.ages[age]);
         json->endArray();
     }
     TRACE_EVENT_OBJECT_SNAPSHOT_WITH_ID(TRACE_DISABLED_BY_DEFAULT("blink_gc"), statsName, this, json.release());
 }
 #endif
 
+#if ENABLE(ASSERT)
+void ThreadState::incrementSelfKeepAliveAllocations()
+{
+    if (!ThreadState::current()->isMainThread())

[haraken, 2015/08/06 10:30:59]

Is there any reason we want to limit the check only to the main thread? (I agree
that other threads will hit ASSERT(persistentCount == 0) when they get detached
but it would be better to have more specific checks to help identify a problem.)

[sof, 2015/08/06 10:40:29]

The reason is that there's protection for thread shutdown wrt thread-local
Persistent<>s.

(I don't think we have non-main thread SelfKeepAlive<>s uses atm.)

[haraken, 2015/08/06 10:42:08]

You could just add ThreadState::m_selfKeepAliveCount and assert the zero-ness in
cleanup() without having any isMainThread() checks.

[sof, 2015/08/06 10:59:01]

You certainly could, but I wonder if we should try to improve the debuggability
of the current assert (ASSERT(!currentCount)) instead.

[haraken, 2015/08/06 11:01:19]

I don't have a strong opinion. I don't think it's a worth but there would be no
reason to limit the check to the main thread :)

[sof, 2015/08/06 12:21:15]

I've added a debugger-friendly entry point to PersistentRegion to dump the live
persistents - worth having?

+        return;
+
+    s_selfKeepAliveAllocationsOnMainThread++;
+}
+
+void ThreadState::decrementSelfKeepAliveAllocations()
+{
+    if (!ThreadState::current()->isMainThread())
+        return;
+
+    ASSERT(s_selfKeepAliveAllocationsOnMainThread > 0);
+    s_selfKeepAliveAllocationsOnMainThread--;
+}
+#endif
+
 } // namespace blink