Oilpan: DatabaseThread::m_thread should not get destructed during sweeping

Source/modules/webdatabase/DatabaseThread.cpp

Index: Source/modules/webdatabase/DatabaseThread.cpp
diff --git a/Source/modules/webdatabase/DatabaseThread.cpp b/Source/modules/webdatabase/DatabaseThread.cpp
index 92bd6faaafd082a2364a2d812cd30e194366c29b..b72dd95a21087ea55ba769604af7242b65df9de9 100644
--- a/Source/modules/webdatabase/DatabaseThread.cpp
+++ b/Source/modules/webdatabase/DatabaseThread.cpp
@@ -43,7 +43,6 @@ namespace blink {
 DatabaseThread::DatabaseThread()
     : m_transactionClient(adoptPtr(new SQLTransactionClient()))
     , m_transactionCoordinator(adoptPtrWillBeNoop(new SQLTransactionCoordinator()))
-    , m_cleanupSync(0)
     , m_terminationRequested(false)
 {
 }
@@ -51,13 +50,7 @@ DatabaseThread::DatabaseThread()
 DatabaseThread::~DatabaseThread()
 {
     ASSERT(m_openDatabaseSet.isEmpty());
-    // Oilpan: The database thread must have finished its cleanup tasks before
-    // the following clear(). Otherwise, WebThread destructor blocks the caller
-    // thread, and causes a deadlock with ThreadState cleanup.
-    // DatabaseContext::stop() asks the database thread to close all of
-    // databases, and wait until GC heap cleanup of the database thread. So we
-    // can safely destruct WebThread here.
-    m_thread.clear();
+    ASSERT(!m_thread);
 }
 
 void DatabaseThread::trace(Visitor* visitor)
@@ -81,14 +74,17 @@ void DatabaseThread::setupDatabaseThread()
     m_thread->attachGC();
 }
 
-void DatabaseThread::requestTermination(TaskSynchronizer *cleanupSync)
+void DatabaseThread::terminate()
 {
-    MutexLocker lock(m_terminationRequestedMutex);
-    ASSERT(!m_terminationRequested);
-    m_terminationRequested = true;
-    m_cleanupSync = cleanupSync;
-    WTF_LOG(StorageAPI, "DatabaseThread %p was asked to terminate\n", this);
-    m_thread->postTask(new Task(WTF::bind(&DatabaseThread::cleanupDatabaseThread, this)));
+    {
+        MutexLocker lock(m_terminationRequestedMutex);
+        ASSERT(!m_terminationRequested);
+        m_terminationRequested = true;
+        WTF_LOG(StorageAPI, "DatabaseThread %p was asked to terminate\n", this);
+        m_thread->postTask(new Task(WTF::bind(&DatabaseThread::cleanupDatabaseThread, this)));
+    }
+    // The WebThread destructor blocks until the database thread processes the cleanup task.
+    m_thread.clear();

[Mads Ager (chromium), 2014/09/23 14:09:08]

Hmm, how does this work. You post the cleanupDatabaseThread task and then you
set m_thread to 0. However, the cleanupDatabaseThread will then try to enqueue a
new task using m_thread. That sounds like something that could/will lead to a
null ptr crash?

 }
 
 bool DatabaseThread::terminationRequested(TaskSynchronizer* taskSynchronizer) const
@@ -126,8 +122,6 @@ void DatabaseThread::cleanupDatabaseThread()
 void DatabaseThread::cleanupDatabaseThreadCompleted()
 {
     m_thread->detachGC();
-    if (m_cleanupSync) // Someone wanted to know when we were done cleaning up.
-        m_cleanupSync->taskCompleted();
 }
 
 void DatabaseThread::recordDatabaseOpen(DatabaseBackend* database)