Title: [247649] trunk/Source/WebCore
Revision
247649
Author
[email protected]
Date
2019-07-19 11:06:09 -0700 (Fri, 19 Jul 2019)

Log Message

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:

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (247648 => 247649)


--- trunk/Source/WebCore/ChangeLog	2019-07-19 16:56:00 UTC (rev 247648)
+++ trunk/Source/WebCore/ChangeLog	2019-07-19 18:06:09 UTC (rev 247649)
@@ -1,3 +1,29 @@
+2019-07-19  Sihui Liu  <[email protected]>
+
+        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  <[email protected]>
 
         [LFC][IFC] Add partial content handling

Modified: trunk/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.cpp (247648 => 247649)


--- trunk/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.cpp	2019-07-19 16:56:00 UTC (rev 247648)
+++ trunk/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.cpp	2019-07-19 18:06:09 UTC (rev 247649)
@@ -628,30 +628,51 @@
     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()
 {
     ASSERT(m_currentOpenDBRequest);
@@ -2191,6 +2212,11 @@
     for (auto& connection : openDatabaseConnections)
         connectionClosedFromServer(*connection);
 
+    if (m_versionChangeDatabaseConnection) {
+        connectionClosedFromServer(*m_versionChangeDatabaseConnection);
+        m_versionChangeDatabaseConnection = nullptr;
+    }
+
     // Cancel the operation timer
     m_operationAndTransactionTimer.stop();
 

Modified: trunk/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.h (247648 => 247649)


--- trunk/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.h	2019-07-19 16:56:00 UTC (rev 247648)
+++ trunk/Source/WebCore/Modules/indexeddb/server/UniqueIDBDatabase.h	2019-07-19 18:06:09 UTC (rev 247649)
@@ -191,6 +191,7 @@
     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 @@
     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();
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to