Tentative fix of a crash in Oilpan GC after a database thread termination.

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 96 matching lines @@
     // Close the databases that we ran transactions on. This ensures that if any transactions are still open, they are rolled back and we don't leave the database in an
     // inconsistent or locked state.
     if (m_openDatabaseSet.size() > 0) {
         // As the call to close will modify the original set, we must take a copy to iterate over.
         HeapHashSet<Member<Database> > openSetCopy;
         openSetCopy.swap(m_openDatabaseSet);
         HeapHashSet<Member<Database> >::iterator end = openSetCopy.end();
         for (HeapHashSet<Member<Database> >::iterator it = openSetCopy.begin(); it != end; ++it)
             (*it)->close();
     }
+    m_openDatabaseSet.clear();
 
     m_thread->postTask(new Task(WTF::bind(&DatabaseThread::cleanupDatabaseThreadCompleted, this)));

[haraken, 2014/10/15 08:20:18]

Just help me understand: Why do we need to post this task? Can't we just call
cleanupDatabaseThreadCompleted() here?

[tkent, 2014/10/15 08:41:22]

I don't remember the reason.  Anyway, it's unrelated to this CL.

 }
 
 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(Database* database)
 {
     ASSERT(isDatabaseThread());
     ASSERT(database);
     ASSERT(!m_openDatabaseSet.contains(database));
-    m_openDatabaseSet.add(database);
+    MutexLocker lock(m_terminationRequestedMutex);
+    if (!m_terminationRequested)
+        m_openDatabaseSet.add(database);
 }
 
 void DatabaseThread::recordDatabaseClosed(Database* database)
 {
     ASSERT(isDatabaseThread());
     ASSERT(database);
     ASSERT(m_terminationRequested || m_openDatabaseSet.contains(database));
     m_openDatabaseSet.remove(database);

[haraken, 2014/10/15 08:20:18]

Don't you need to add a MutexLocker and check !m_terminationRequested here as
well?

[tkent, 2014/10/15 08:41:22]

No. This function is supposed to be called after a termination request.

[tkent, 2014/10/15 08:43:47]

This HeapHashSet::remove() is harmless even if m_terminationRequested is true,
and mutex lock is  not a light operation.

 }
 
 bool DatabaseThread::isDatabaseOpen(Database* database)
 {
     ASSERT(isDatabaseThread());
     ASSERT(database);
     MutexLocker lock(m_terminationRequestedMutex);
     return !m_terminationRequested && m_openDatabaseSet.contains(database);
 }
 
 void DatabaseThread::scheduleTask(PassOwnPtr<DatabaseTask> task)
 {
     ASSERT(m_thread);
     ASSERT(!terminationRequested());
     // WebThread takes ownership of the task.
     m_thread->postTask(task.leakPtr());
 }
 
 } // namespace blink