Finish completeSweep before shutting down V8

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 191 matching lines @@
 #endif
 
 void ThreadState::attachMainThread()
 {
     RELEASE_ASSERT(!Heap::s_shutdownCalled);
     MutexLocker locker(threadAttachMutex());
     ThreadState* state = new (s_mainThreadStateStorage) ThreadState();
     attachedThreads().add(state);
 }
 
+
+void ThreadState::stopMainThread()
+{
+    ASSERT(isMainThread());
+
+    // Finish sweeping before shutting down V8. Otherwise, some destructor
+    // may access V8 and cause crashes.
+    completeSweep();

[haraken, 2016/03/10 05:42:23]

Maybe we should add enterGCForbidenScope() before finishing stopMainThread() to
make sure that no GC happens before detaching the main thread. (I don't have a
strong opinion.)

[haraken, 2016/03/10 05:42:23]

Another option is just to remove completeSweep(). There is no reason we have to
complete sweeping before shutting down the main thread. However, I prefer
completing sweeping just in case.

In production, blink::shutdown would rarely be called because the renderer is
killed immediately in common cases. So having completeSweep() in the shutdown
sequence won't matter for real-world performance.

+}
+
 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 == ThreadState::current());
     ASSERT(state->checkThread());
     // You must call unregisterTraceDOMWrappers before detaching
     // the main thread.
     ASSERT(!state->m_isolate);
+    ASSERT(!state->isSweepingInProgress());
 
-    // 1. Finish sweeping.
-    state->completeSweep();
-    {
-        SafePointAwareMutexLocker locker(threadAttachMutex(), BlinkGC::NoHeapPointersOnStack);
+    SafePointAwareMutexLocker locker(threadAttachMutex(), BlinkGC::NoHeapPointersOnStack);
 
-        // 2. Add the main thread's heap pages to the orphaned pool.
-        state->cleanupPages();
+    // 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();
-    }
+    // Detach the main thread.
+    ASSERT(attachedThreads().contains(state));
+    attachedThreads().remove(state);
+    state->~ThreadState();
 }
 
 void ThreadState::attach()
 {
     RELEASE_ASSERT(!Heap::s_shutdownCalled);
     MutexLocker locker(threadAttachMutex());
     ThreadState* state = new ThreadState();
     attachedThreads().add(state);
 }
 
@@ skipping 1290 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