2009-03-28 Darin Adler <darin@apple.com>
authordarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 29 Mar 2009 00:16:17 +0000 (00:16 +0000)
committerdarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 29 Mar 2009 00:16:17 +0000 (00:16 +0000)
        Reviewed by Mark Rowe.

        Bug 24914: empty-string assertion crash when running storage tests
        https://bugs.webkit.org/show_bug.cgi?id=24914

        * storage/Database.cpp:
        (WebCore::Database::performOpenAndVerify): Don't store empty version strings
        in the map, since empty strings are per-thread.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@42076 268f45cc-cd09-0410-ab3c-d52691b4dbfc

WebCore/ChangeLog
WebCore/storage/Database.cpp

index 740a98e..79f57ea 100644 (file)
@@ -1,3 +1,14 @@
+2009-03-28  Darin Adler  <darin@apple.com>
+
+        Reviewed by Mark Rowe.
+
+        Bug 24914: empty-string assertion crash when running storage tests
+        https://bugs.webkit.org/show_bug.cgi?id=24914
+
+        * storage/Database.cpp:
+        (WebCore::Database::performOpenAndVerify): Don't store empty version strings
+        in the map, since empty strings are per-thread.
+
 2009-03-28  Dmitry Titov  <dimich@chromium.org>
 
         Reviewed by Dimitri Glazkov.
index 941c49c..b37b9bd 100644 (file)
@@ -426,18 +426,23 @@ bool Database::performOpenAndVerify(ExceptionCode& e)
         }
     }
 
-
     String currentVersion;
     {
         MutexLocker locker(guidMutex());
-        currentVersion = guidToVersionMap().get(m_guid);
 
-        if (currentVersion.isNull())
-            LOG(StorageAPI, "Current cached version for guid %i is null", m_guid);
-        else
-            LOG(StorageAPI, "Current cached version for guid %i is %s", m_guid, currentVersion.ascii().data());
+        // Note: It is not safe to put an empty string into the guidToVersionMap() map.
+        // That's because the map is cross-thread, but empty strings are per-thread.
+        // The copy() function makes a version of the string you can use on the current
+        // thread, but we need a string we can keep in a cross-thread data structure.
+        // FIXME: This is a quite-awkward restriction to have to program with.
 
-        if (currentVersion.isNull()) {
+        GuidVersionMap::iterator entry = guidToVersionMap().find(m_guid);
+        if (entry != guidToVersionMap().end()) {
+            // Map null string to empty string (see comment above).
+            currentVersion = entry->second.isNull() ? String("") : entry->second;
+            LOG(StorageAPI, "Current cached version for guid %i is %s", m_guid, currentVersion.ascii().data());
+        } else {
+            LOG(StorageAPI, "No cached version for guid %i", m_guid);
             if (!getVersionFromDatabase(currentVersion)) {
                 LOG_ERROR("Failed to get current version from database %s", databaseDebugName().ascii().data());
                 e = INVALID_STATE_ERR;
@@ -452,11 +457,11 @@ bool Database::performOpenAndVerify(ExceptionCode& e)
                     e = INVALID_STATE_ERR;
                     return false;
                 }
-
                 currentVersion = m_expectedVersion;
             }
 
-            guidToVersionMap().set(m_guid, currentVersion.copy());
+            // Map empty string to null string (see comment above).
+            guidToVersionMap().set(m_guid, currentVersion.isEmpty() ? String() : currentVersion.copy());
         }
     }