Oilpan: We need to collect all garbage before shutting down the worker thread

Source/core/workers/WorkerThread.cpp

 /*
  * Copyright (C) 2008 Apple 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:
  * 1. Redistributions of source code must retain the above copyright
  *    notice, this list of conditions and the following disclaimer.
  * 2. Redistributions in binary form must reproduce the above copyright
  *    notice, this list of conditions and the following disclaimer in the
@@ skipping 119 matching lines @@
     m_workerReportingProxy.workerGlobalScopeStarted(m_workerGlobalScope.get());
 
     WorkerScriptController* script = m_workerGlobalScope->script();
     InspectorInstrumentation::willEvaluateWorkerScript(workerGlobalScope(), startMode);
     script->evaluate(ScriptSourceCode(sourceCode, scriptURL));
 
     runEventLoop();
 
     ThreadIdentifier threadID = m_threadID;
 
+    // We need to collect all garbage before checking that the WorkerThread has the last reference
+    // to the WorkerGlobalScope. We don't need to call V8's GC because all wrappers of the worker thread
+    // are stored in DOMWrapperMap (i.e., no wrappers of the worker thread are stored in ScriptWrappable)
+    // and they are already cleared out in ~WorkerScriptController(). In short, we just need to call
+    // oilpan's GC to collect all garbage.
+    Heap::collectAllGarbage(ThreadState::NoHeapPointersOnStack);

[haraken, 2014/03/05 05:08:35]

Actually, I'll remove this code soon because the following ASSERT will be
removed by sof's CL that moves WorkerGlobalScope to the heap.

After removing the ASSERT, we just need to make sure that the thread heap is
empty at the end of ThreadState::cleanup(). In other words, we just need to make
sure that all garbage is collected before checking that the thread heap is
empty. However, this is already done in ThreadState::cleanup().

In conclusion, once we remove the ASSERT, we can just remove
Heap::collectAllGarbage().

[Mads Ager (chromium), 2014/03/05 07:40:34]

We should just #ifdef out the ASSERT now so we don't have to introduce the
comment above. Then either you or sof will merge that. :-)

[haraken, 2014/03/05 08:50:26]

Done.

     ASSERT(m_workerGlobalScope->hasOneRef());
 
     // The below assignment will destroy the context, which will in turn notify messaging proxy.
     // We cannot let any objects survive past thread exit, because no other thread will run GC or otherwise destroy them.
     m_workerGlobalScope = nullptr;
 
-    // Cleanup thread heap which causes all objects to be finalized.
-    // After this call thread heap must be empty.
-    ThreadState::current()->cleanup();
+    ThreadState::detach();
 
     // Clean up PlatformThreadData before WTF::WTFThreadData goes away!
     PlatformThreadData::current().destroy();
 
     // The thread object may be already destroyed from notification now, don't try to access "this".
     detachThread(threadID);
-
-    ThreadState::detach();
 }
 
 void WorkerThread::runEventLoop()
 {
     // Does not return until terminated.
     m_runLoop.run();
 }
 
 class WorkerThreadShutdownFinishTask : public ExecutionContextTask {
 public:
@@ skipping 81 matching lines @@
 void WorkerThread::releaseFastMallocFreeMemoryInAllThreads()
 {
     MutexLocker lock(threadSetMutex());
     HashSet<WorkerThread*>& threads = workerThreads();
     HashSet<WorkerThread*>::iterator end = threads.end();
     for (HashSet<WorkerThread*>::iterator it = threads.begin(); it != end; ++it)
         (*it)->runLoop().postTask(adoptPtr(new ReleaseFastMallocFreeMemoryTask));
 }
 
 } // namespace WebCore