[oilpan]: RFC: Change DatabaseBackendBase::performOpenAndVerify locking

Source/modules/webdatabase/DatabaseBackendBase.cpp

Index: Source/modules/webdatabase/DatabaseBackendBase.cpp
diff --git a/Source/modules/webdatabase/DatabaseBackendBase.cpp b/Source/modules/webdatabase/DatabaseBackendBase.cpp
index 4cc6b32069273546e603fb907205936a775a5d94..5b0bb97595b7c2c8cb9a6b115f986a8043ea0c31 100644
--- a/Source/modules/webdatabase/DatabaseBackendBase.cpp
+++ b/Source/modules/webdatabase/DatabaseBackendBase.cpp
@@ -329,10 +329,14 @@ bool DatabaseBackendBase::performOpenAndVerify(bool shouldSetVersionInNewDatabas
 
     String currentVersion;
     {
-        MutexLocker locker(guidMutex());

[Vyacheslav Egorov (Google), 2014/06/13 12:49:39]

An alternative less intrusive approach would be to have SafePoint around attempt
to lock this mutex, similar to how we handle thread detachment. 

ThreadState::current()->enterSafePoint(...);
MutexLocker locker(...)
ThreadState::current()->leaveSafePoint();

For the best effect this can be wrapped into the SafePointAwareMutexLocker
class.

We can even have SafePointAwareMutex which would require
SafePointAwareMutexLocker to be used on it and make all database related mutexes
SafePointAware.

-
-        GuidVersionMap::iterator entry = guidToVersionMap().find(m_guid);
-        if (entry != guidToVersionMap().end()) {
+        GuidVersionMap::iterator entry;
+        GuidVersionMap::iterator end;
+        {
+            MutexLocker locker(guidMutex());
+            entry = guidToVersionMap().find(m_guid);
+            end = guidToVersionMap().end();
+        }
+        if (entry != end) {
             // Map null string to empty string (see updateGuidVersionMap()).
             currentVersion = entry->value.isNull() ? emptyString() : entry->value.isolatedCopy();
             WTF_LOG(StorageAPI, "Current cached version for guid %i is %s", m_guid, currentVersion.ascii().data());
@@ -348,6 +352,7 @@ bool DatabaseBackendBase::performOpenAndVerify(bool shouldSetVersionInNewDatabas
             String versionFromDatabase;
             if (getVersionFromDatabase(versionFromDatabase, false)) {
                 currentVersion = versionFromDatabase;
+                MutexLocker locker(guidMutex());
                 updateGuidVersionMap(m_guid, currentVersion);
             }
             m_sqliteDatabase.setBusyTimeout(maxSqliteBusyWaitTime);
@@ -395,8 +400,24 @@ bool DatabaseBackendBase::performOpenAndVerify(bool shouldSetVersionInNewDatabas
                 }
                 currentVersion = m_expectedVersion;
             }
-            updateGuidVersionMap(m_guid, currentVersion);
-            transaction.commit();
+
+            {
+                MutexLocker locker(guidMutex());
+                entry = guidToVersionMap().find(m_guid);
+                end = guidToVersionMap().end();
+                // We always update the map with the version when at this point.
+                updateGuidVersionMap(m_guid, currentVersion);
+            }
+
+            // Only commit the transaction if no other thread has already created the table, etc.
+            // We use the presence of an entry in the guidToVersionMap to determine if this thread
+            // is first.
+            if (entry != end) {
+                transaction.commit();
+            } else {
+                // Some other thread has already created table, set the version, etc.
+                transaction.rollback();
+            }
         }
     }