IndexedDB: error in starting version change transaction may be neglected
authorsihui_liu@apple.com <sihui_liu@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 19 Jul 2019 18:06:09 +0000 (18:06 +0000)
committersihui_liu@apple.com <sihui_liu@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 19 Jul 2019 18:06:09 +0000 (18:06 +0000)
https://bugs.webkit.org/show_bug.cgi?id=199818
<rdar://problem/52925738>

Reviewed by Brady Eidson.

For version change transaction, IDBServer didn't wait the result of beginTransaction on the background thread
before giving the IDBClient the result of open request. In this case, beginTransaction may fail to update the
DatabaseVersion in database file or set m_originalDatabaseInfoBeforeVersionChange, but the transaction was
marked as started. When we later set m_databaseInfo with m_originalDatabaseInfoBeforeVersionChange,
m_databaseInfo could become nullptr.

To write a test for this, we will need to simulate an SQLite error. I manually tested this by crafting the
SQLiteStatement in beginTransaction, making it an invalid statement, and verified that error event, instead of
ungradeneeded event is dispatched to the IDBRequest.

* Modules/indexeddb/server/UniqueIDBDatabase.cpp:
(WebCore::IDBServer::UniqueIDBDatabase::startVersionChangeTransaction):
(WebCore::IDBServer::UniqueIDBDatabase::performStartVersionChangeTransaction):
(WebCore::IDBServer::UniqueIDBDatabase::didPerformStartVersionChangeTransaction):
(WebCore::IDBServer::UniqueIDBDatabase::immediateCloseForUserDelete):
(WebCore::IDBServer::UniqueIDBDatabase::beginTransactionInBackingStore): Deleted.
* Modules/indexeddb/server/UniqueIDBDatabase.h:

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

Source/WebCore/ChangeLog
Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.cpp
Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.h

index cff5298..304ecc1 100644 (file)
@@ -1,3 +1,29 @@
+2019-07-19  Sihui Liu  <sihui_liu@apple.com>
+
+        IndexedDB: error in starting version change transaction may be neglected
+        https://bugs.webkit.org/show_bug.cgi?id=199818
+        <rdar://problem/52925738>
+
+        Reviewed by Brady Eidson.
+
+        For version change transaction, IDBServer didn't wait the result of beginTransaction on the background thread 
+        before giving the IDBClient the result of open request. In this case, beginTransaction may fail to update the 
+        DatabaseVersion in database file or set m_originalDatabaseInfoBeforeVersionChange, but the transaction was
+        marked as started. When we later set m_databaseInfo with m_originalDatabaseInfoBeforeVersionChange, 
+        m_databaseInfo could become nullptr.
+
+        To write a test for this, we will need to simulate an SQLite error. I manually tested this by crafting the 
+        SQLiteStatement in beginTransaction, making it an invalid statement, and verified that error event, instead of 
+        ungradeneeded event is dispatched to the IDBRequest.
+
+        * Modules/indexeddb/server/UniqueIDBDatabase.cpp:
+        (WebCore::IDBServer::UniqueIDBDatabase::startVersionChangeTransaction):
+        (WebCore::IDBServer::UniqueIDBDatabase::performStartVersionChangeTransaction):
+        (WebCore::IDBServer::UniqueIDBDatabase::didPerformStartVersionChangeTransaction):
+        (WebCore::IDBServer::UniqueIDBDatabase::immediateCloseForUserDelete):
+        (WebCore::IDBServer::UniqueIDBDatabase::beginTransactionInBackingStore): Deleted.
+        * Modules/indexeddb/server/UniqueIDBDatabase.h:
+
 2019-07-19  Zalan Bujtas  <zalan@apple.com>
 
         [LFC][IFC] Add partial content handling
index b94b82c..7304974 100644 (file)
@@ -628,28 +628,49 @@ void UniqueIDBDatabase::startVersionChangeTransaction()
     ASSERT(m_currentOpenDBRequest->isOpenRequest());
     ASSERT(m_versionChangeDatabaseConnection);
 
-    auto operation = WTFMove(m_currentOpenDBRequest);
-
-    uint64_t requestedVersion = operation->requestData().requestedVersion();
+    uint64_t requestedVersion = m_currentOpenDBRequest->requestData().requestedVersion();
     if (!requestedVersion)
         requestedVersion = m_databaseInfo->version() ? m_databaseInfo->version() : 1;
 
-    addOpenDatabaseConnection(*m_versionChangeDatabaseConnection);
-
     m_versionChangeTransaction = &m_versionChangeDatabaseConnection->createVersionChangeTransaction(requestedVersion);
-    m_databaseInfo->setVersion(requestedVersion);
-
     m_inProgressTransactions.set(m_versionChangeTransaction->info().identifier(), m_versionChangeTransaction);
-    postDatabaseTask(createCrossThreadTask(*this, &UniqueIDBDatabase::beginTransactionInBackingStore, m_versionChangeTransaction->info()));
 
-    auto result = IDBResultData::openDatabaseUpgradeNeeded(operation->requestData().requestIdentifier(), *m_versionChangeTransaction);
-    operation->connection().didOpenDatabase(result);
+    postDatabaseTask(createCrossThreadTask(*this, &UniqueIDBDatabase::performStartVersionChangeTransaction, m_versionChangeTransaction->info()));
 }
 
-void UniqueIDBDatabase::beginTransactionInBackingStore(const IDBTransactionInfo& info)
+void UniqueIDBDatabase::performStartVersionChangeTransaction(const IDBTransactionInfo& info)
 {
-    LOG(IndexedDB, "(db) UniqueIDBDatabase::beginTransactionInBackingStore");
-    m_backingStore->beginTransaction(info);
+    LOG(IndexedDB, "(db) UniqueIDBDatabase::performStartVersionChangeTransaction");
+
+    IDBError error = m_backingStore->beginTransaction(info);
+    postDatabaseTaskReply(createCrossThreadTask(*this, &UniqueIDBDatabase::didPerformStartVersionChangeTransaction, error));
+}
+
+void UniqueIDBDatabase::didPerformStartVersionChangeTransaction(const IDBError& error)
+{
+    LOG(IndexedDB, "(main) UniqueIDBDatabase::didPerformStartVersionChangeTransaction");
+
+    // Open request may already be canceled by client or user, or connection to client is lost.
+    if (!m_versionChangeDatabaseConnection)
+        return;
+    
+    ASSERT(m_currentOpenDBRequest);
+    ASSERT(m_versionChangeTransaction);
+    auto operation = WTFMove(m_currentOpenDBRequest);
+    IDBResultData result;
+    if (error.isNull()) {
+        addOpenDatabaseConnection(*m_versionChangeDatabaseConnection);
+        m_databaseInfo->setVersion(m_versionChangeTransaction->info().newVersion());
+        result = IDBResultData::openDatabaseUpgradeNeeded(operation->requestData().requestIdentifier(), *m_versionChangeTransaction);
+        operation->connection().didOpenDatabase(result);
+    } else {
+        m_versionChangeDatabaseConnection->abortTransactionWithoutCallback(*m_versionChangeTransaction);
+        m_versionChangeDatabaseConnection = nullptr;
+        result = IDBResultData::error(operation->requestData().requestIdentifier(), error);
+        operation->connection().didOpenDatabase(result);
+    }
+
+    invokeOperationAndTransactionTimer();
 }
 
 void UniqueIDBDatabase::maybeNotifyConnectionsOfVersionChange()
@@ -2191,6 +2212,11 @@ void UniqueIDBDatabase::immediateCloseForUserDelete()
     for (auto& connection : openDatabaseConnections)
         connectionClosedFromServer(*connection);
 
+    if (m_versionChangeDatabaseConnection) {
+        connectionClosedFromServer(*m_versionChangeDatabaseConnection);
+        m_versionChangeDatabaseConnection = nullptr;
+    }
+
     // Cancel the operation timer
     m_operationAndTransactionTimer.stop();
 
index 669b2d6..8c2a1b3 100644 (file)
@@ -191,6 +191,7 @@ private:
     void performIterateCursor(uint64_t callbackIdentifier, const IDBResourceIdentifier& transactionIdentifier, const IDBResourceIdentifier& cursorIdentifier, const IDBIterateCursorData&);
     void performPrefetchCursor(const IDBResourceIdentifier& transactionIdentifier, const IDBResourceIdentifier& cursorIdentifier);
 
+    void performStartVersionChangeTransaction(const IDBTransactionInfo&);
     void performActivateTransactionInBackingStore(uint64_t callbackIdentifier, const IDBTransactionInfo&);
     void performUnconditionalDeleteBackingStore();
     void shutdownForClose();
@@ -214,6 +215,8 @@ private:
     void didPerformIterateCursor(uint64_t callbackIdentifier, const IDBError&, const IDBGetResult&);
     void didPerformCommitTransaction(uint64_t callbackIdentifier, const IDBError&, const IDBResourceIdentifier& transactionIdentifier);
     void didPerformAbortTransaction(uint64_t callbackIdentifier, const IDBError&, const IDBResourceIdentifier& transactionIdentifier);
+
+    void didPerformStartVersionChangeTransaction(const IDBError&);
     void didPerformActivateTransactionInBackingStore(uint64_t callbackIdentifier, const IDBError&);
     void didPerformUnconditionalDeleteBackingStore();
     void didShutdownForClose();