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

Source/modules/webdatabase/DatabaseThread.cpp

 /*
  * Copyright (C) 2007, 2008, 2013 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
@@ skipping 25 matching lines @@
 #include "platform/Logging.h"
 #include "platform/heap/glue/MessageLoopInterruptor.h"
 #include "platform/heap/glue/PendingGCRunner.h"
 #include "public/platform/Platform.h"
 
 namespace blink {
 
 DatabaseThread::DatabaseThread()
     : m_transactionClient(adoptPtr(new SQLTransactionClient()))
     , m_transactionCoordinator(adoptPtrWillBeNoop(new SQLTransactionCoordinator()))
-    , m_cleanupSync(0)
     , m_terminationRequested(false)
 {
 }
 
 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)
 {
 #if ENABLE(OILPAN)
     visitor->trace(m_openDatabaseSet);
     visitor->trace(m_transactionCoordinator);
 #endif
 }
 
 void DatabaseThread::start()
 {
     if (m_thread)
         return;
     m_thread = WebThreadSupportingGC::create("WebCore: Database");
     m_thread->postTask(new Task(WTF::bind(&DatabaseThread::setupDatabaseThread, this)));
 }
 
 void DatabaseThread::setupDatabaseThread()
 {
     m_thread->attachGC();
 }
 
-void DatabaseThread::requestTermination(TaskSynchronizer *cleanupSync)
+void DatabaseThread::terminate()
 {
     MutexLocker lock(m_terminationRequestedMutex);

[Mads Ager (chromium), 2014/09/23 13:44:37]

I think we should limit the scope of this lock so it is only held while updating
terminationRequested and posting the task. We don't want to hold the lock while
waiting for the thread to terminate.

{
    MutexLocker lock(...);
    ...
    m_thread->postTask(...);
}
m_thread->clear();

?

     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)));
+    // The WebThread destructor blocks until the database thread processes the cleanup task.
+    m_thread.clear();
 }
 
 bool DatabaseThread::terminationRequested(TaskSynchronizer* taskSynchronizer) const
 {
 #if ENABLE(ASSERT)
     if (taskSynchronizer)
         taskSynchronizer->setHasCheckedForTermination();
 #endif
 
     MutexLocker lock(m_terminationRequestedMutex);
@@ skipping 17 matching lines @@
         for (WillBeHeapHashSet<RefPtrWillBeMember<DatabaseBackend> >::iterator it = openSetCopy.begin(); it != end; ++it)
             (*it)->close();
     }
 
     m_thread->postTask(new Task(WTF::bind(&DatabaseThread::cleanupDatabaseThreadCompleted, this)));
 }
 
 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)
 {
     ASSERT(isDatabaseThread());
     ASSERT(database);
     ASSERT(!m_openDatabaseSet.contains(database));
     m_openDatabaseSet.add(database);
 }
 
@@ skipping 15 matching lines @@
 
 void DatabaseThread::scheduleTask(PassOwnPtr<DatabaseTask> task)
 {
     ASSERT(m_thread);
     ASSERT(!task->hasSynchronizer() || task->hasCheckedForTermination());
     // WebThread takes ownership of the task.
     m_thread->postTask(task.leakPtr());
 }
 
 } // namespace blink