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

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 f751bba454a88eeb6e42e80aaae8a002596dcfe4..0f65d307702d0f0c9242eb3b6a5399b2e718c00f 100644
--- a/Source/platform/heap/ThreadState.cpp
+++ b/Source/platform/heap/ThreadState.cpp
@@ -76,6 +76,9 @@ 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()
 {
@@ -166,6 +169,9 @@ void ThreadState::attachMainThread()
     MutexLocker locker(threadAttachMutex());
     ThreadState* state = new(s_mainThreadStateStorage) ThreadState();
     attachedThreads().add(state);
+#if ENABLE(ASSERT)
+    s_selfKeepAliveAllocationsOnMainThread = 0;
+#endif
 }
 
 void ThreadState::detachMainThread()
@@ -189,6 +195,8 @@ void ThreadState::detachMainThread()
         attachedThreads().remove(state);
         state->~ThreadState();
     }
+    // Catch out any self-referential leaks created by the main thread.
+    ASSERT(s_selfKeepAliveAllocationsOnMainThread == 0);
     shutdownHeapIfNecessary();
 }
 
@@ -1537,4 +1545,23 @@ void ThreadState::reportMarkSweepStats(const char* statsName, const ClassAgeCoun
 }
 #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