Reviewed by Tim Hatcher.
authordarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 4 Feb 2008 17:25:10 +0000 (17:25 +0000)
committerdarin@apple.com <darin@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 4 Feb 2008 17:25:10 +0000 (17:25 +0000)
        - possible fix for <rdar://problem/5714030> Crash in Database::deliverAllPendingCallbacks()
          reloading a page quickly

        I don't fully understand the cause of the crash, but I think this might
        be a helpful change.

        * platform/sql/SQLiteTransaction.cpp:
        (WebCore::SQLiteTransaction::commit): If the commit fails, don't leave this
        transaction and database both marked as "still in progress". As far as I can
        tell this does no good, and also seems to do harm.
        (WebCore::SQLiteTransaction::rollback): Ditto.

        * storage/Database.cpp:
        (WebCore::Database::performTransactionStep): Add some assertions to
        detect databases stuck in the "transaction in progress" state.
        * storage/SQLTransaction.cpp:
        (WebCore::SQLTransaction::openTransactionAndPreflight): Ditto.
        (WebCore::SQLTransaction::postflightAndCommit): Ditto.
        (WebCore::SQLTransaction::cleanupAfterTransactionErrorCallback): Ditto.

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

WebCore/ChangeLog
WebCore/platform/sql/SQLiteTransaction.cpp
WebCore/storage/Database.cpp
WebCore/storage/SQLTransaction.cpp

index 3146197..02d03c3 100644 (file)
@@ -1,5 +1,29 @@
 2008-02-04  Darin Adler  <darin@apple.com>
 
+        Reviewed by Tim Hatcher.
+
+        - possible fix for <rdar://problem/5714030> Crash in Database::deliverAllPendingCallbacks()
+          reloading a page quickly
+
+        I don't fully understand the cause of the crash, but I think this might
+        be a helpful change.
+
+        * platform/sql/SQLiteTransaction.cpp:
+        (WebCore::SQLiteTransaction::commit): If the commit fails, don't leave this
+        transaction and database both marked as "still in progress". As far as I can
+        tell this does no good, and also seems to do harm.
+        (WebCore::SQLiteTransaction::rollback): Ditto.
+
+        * storage/Database.cpp:
+        (WebCore::Database::performTransactionStep): Add some assertions to
+        detect databases stuck in the "transaction in progress" state.
+        * storage/SQLTransaction.cpp:
+        (WebCore::SQLTransaction::openTransactionAndPreflight): Ditto.
+        (WebCore::SQLTransaction::postflightAndCommit): Ditto.
+        (WebCore::SQLTransaction::cleanupAfterTransactionErrorCallback): Ditto.
+
+2008-02-04  Darin Adler  <darin@apple.com>
+
         Reviewed by Geoff.
 
         - fix <rdar://problem/5715692> REGRESSION (r28570): JavaScript window.scrollTo()
index b502662..41d27c2 100644 (file)
@@ -55,10 +55,9 @@ void SQLiteTransaction::commit()
 {
     if (m_inProgress) {
         ASSERT(m_db.m_transactionInProgress);
-        if (m_db.executeCommand("COMMIT;")) {
-            m_inProgress = false;
-            m_db.m_transactionInProgress = false;
-        }
+        m_db.executeCommand("COMMIT;");
+        m_inProgress = false;
+        m_db.m_transactionInProgress = false;
     }
 }
 
@@ -66,10 +65,9 @@ void SQLiteTransaction::rollback()
 {
     if (m_inProgress) {
         ASSERT(m_db.m_transactionInProgress);
-        if (m_db.executeCommand("ROLLBACK;")) {
-            m_inProgress = false;
-            m_db.m_transactionInProgress = false;
-        }
+        m_db.executeCommand("ROLLBACK;");
+        m_inProgress = false;
+        m_db.m_transactionInProgress = false;
     }
 }
     
index 2458485..649b276 100644 (file)
@@ -453,6 +453,7 @@ void Database::performTransactionStep()
 
     {
         MutexLocker locker(m_transactionMutex);
+        ASSERT(!m_sqliteDatabase.transactionInProgress());
         m_currentTransaction = 0;
 
         if (m_transactionQueue.size())
index 2fe7d30..900ea27 100644 (file)
@@ -115,6 +115,8 @@ void SQLTransaction::performPendingCallback()
 
 void SQLTransaction::openTransactionAndPreflight()
 {
+    ASSERT(!m_database->m_sqliteDatabase.transactionInProgress());
+
     LOG(StorageAPI, "Opening and preflighting transaction %p", this);
     
     // FIXME: This is a glaring bug that gives each database in an origin the full quota of that origin
@@ -132,6 +134,7 @@ void SQLTransaction::openTransactionAndPreflight()
     
     // Transaction Steps 1+2 - Open a transaction to the database, jumping to the error callback if that fails
     if (!m_sqliteTransaction->inProgress()) {
+        ASSERT(!m_database->m_sqliteDatabase.transactionInProgress());
         m_sqliteTransaction.clear();
         m_transactionError = new SQLError(0, "unable to open a transaction to the database");
         handleTransactionError(false);
@@ -140,6 +143,7 @@ void SQLTransaction::openTransactionAndPreflight()
     
     // Transaction Steps 3 - Peform preflight steps, jumping to the error callback if they fail
     if (m_wrapper && !m_wrapper->performPreflight(this)) {
+        ASSERT(!m_database->m_sqliteDatabase.transactionInProgress());
         m_sqliteTransaction.clear();
         m_transactionError = m_wrapper->sqlError();
         if (!m_transactionError)
@@ -340,6 +344,7 @@ void SQLTransaction::postflightAndCommit()
     
     // Transaction Step 10 - End transaction steps
     // There is no next step
+    ASSERT(!m_database->m_sqliteDatabase.transactionInProgress());
     m_nextStep = 0;
 
     // Now release our callbacks, to break reference cycles.
@@ -400,6 +405,7 @@ void SQLTransaction::cleanupAfterTransactionErrorCallback()
             DatabaseTracker::tracker().scheduleNotifyDatabaseChanged(m_database->m_securityOrigin.get(), m_database->m_name);
         }
         
+        ASSERT(!m_database->m_sqliteDatabase.transactionInProgress());
         m_sqliteTransaction.clear();
     }
     m_database->m_databaseAuthorizer->enable();
@@ -411,6 +417,7 @@ void SQLTransaction::cleanupAfterTransactionErrorCallback()
     }
     
     // Transaction is complete!  There is no next step
+    ASSERT(!m_database->m_sqliteDatabase.transactionInProgress());
     m_nextStep = 0;
 
     // Now release our callbacks, to break reference cycles.