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);
-    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
 {
 #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