[Mac WK2] storage/indexeddb/IDBObject-leak.html is flaky
authorsihui_liu@apple.com <sihui_liu@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 26 Feb 2019 23:38:31 +0000 (23:38 +0000)
committersihui_liu@apple.com <sihui_liu@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 26 Feb 2019 23:38:31 +0000 (23:38 +0000)
https://bugs.webkit.org/show_bug.cgi?id=195036

Reviewed by Geoffrey Garen.

When connection to IDBServer is lost, IDBDatabase in web process should not only stop active transactions, but
also transactions in committing process.

Also, TransactionOpration should clear its perform function when the operation is being completed, otherwise
there is a reference cycle of TransactionOpration.

Covered by existing tests storage/indexeddb/IDBObject-leak.html.

* Modules/indexeddb/IDBDatabase.cpp:
(WebCore::IDBDatabase::connectionToServerLost): notify committing transasctions that connection is lost.
* Modules/indexeddb/IDBTransaction.cpp:
(WebCore::IDBTransaction::connectionClosedFromServer): notify IDBConnectionProxy that transaction ends.
* Modules/indexeddb/client/IDBConnectionProxy.cpp:
(WebCore::IDBClient::IDBConnectionProxy::forgetTransaction): clear finished transactions.
* Modules/indexeddb/client/IDBConnectionProxy.h:
* Modules/indexeddb/client/TransactionOperation.h:
(WebCore::IDBClient::TransactionOperation::doComplete): clear perform function unconditionally when the
operation is in completion process.

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

Source/WebCore/ChangeLog
Source/WebCore/Modules/indexeddb/IDBDatabase.cpp
Source/WebCore/Modules/indexeddb/IDBTransaction.cpp
Source/WebCore/Modules/indexeddb/client/IDBConnectionProxy.cpp
Source/WebCore/Modules/indexeddb/client/IDBConnectionProxy.h
Source/WebCore/Modules/indexeddb/client/TransactionOperation.h

index ea22734..c698cb0 100644 (file)
@@ -1,3 +1,29 @@
+2019-02-26  Sihui Liu  <sihui_liu@apple.com>
+
+        [Mac WK2] storage/indexeddb/IDBObject-leak.html is flaky
+        https://bugs.webkit.org/show_bug.cgi?id=195036
+
+        Reviewed by Geoffrey Garen.
+
+        When connection to IDBServer is lost, IDBDatabase in web process should not only stop active transactions, but 
+        also transactions in committing process.
+
+        Also, TransactionOpration should clear its perform function when the operation is being completed, otherwise 
+        there is a reference cycle of TransactionOpration.
+
+        Covered by existing tests storage/indexeddb/IDBObject-leak.html.
+
+        * Modules/indexeddb/IDBDatabase.cpp:
+        (WebCore::IDBDatabase::connectionToServerLost): notify committing transasctions that connection is lost.
+        * Modules/indexeddb/IDBTransaction.cpp:
+        (WebCore::IDBTransaction::connectionClosedFromServer): notify IDBConnectionProxy that transaction ends.
+        * Modules/indexeddb/client/IDBConnectionProxy.cpp:
+        (WebCore::IDBClient::IDBConnectionProxy::forgetTransaction): clear finished transactions.
+        * Modules/indexeddb/client/IDBConnectionProxy.h:
+        * Modules/indexeddb/client/TransactionOperation.h:
+        (WebCore::IDBClient::TransactionOperation::doComplete): clear perform function unconditionally when the 
+        operation is in completion process. 
+
 2019-02-26  Zalan Bujtas  <zalan@apple.com>
 
         [ContentChangeObserver] clearContentChangeObservers should be internal to ContentChangeObserver class
index 0a09572..b95e63c 100644 (file)
@@ -264,8 +264,12 @@ void IDBDatabase::connectionToServerLost(const IDBError& error)
     m_closePending = true;
     m_closedInServer = true;
 
-    auto transactions = copyToVector(m_activeTransactions.values());
-    for (auto& transaction : transactions)
+    auto activeTransactions = copyToVector(m_activeTransactions.values());
+    for (auto& transaction : activeTransactions)
+        transaction->connectionClosedFromServer(error);
+    
+    auto committingTransactions = copyToVector(m_committingTransactions.values());
+    for (auto& transaction : committingTransactions)
         transaction->connectionClosedFromServer(error);
 
     auto errorEvent = Event::create(m_eventNames.errorEvent, Event::CanBubble::Yes, Event::IsCancelable::No);
index a3995d3..e5a0e88 100644 (file)
@@ -1440,7 +1440,7 @@ void IDBTransaction::connectionClosedFromServer(const IDBError& error)
     LOG(IndexedDB, "IDBTransaction::connectionClosedFromServer - %s", error.message().utf8().data());
 
     m_database->willAbortTransaction(*this);
-    transitionedToFinishing(IndexedDB::TransactionState::Aborting);
+    m_state = IndexedDB::TransactionState::Aborting;
 
     abortInProgressOperations(error);
 
@@ -1454,6 +1454,7 @@ void IDBTransaction::connectionClosedFromServer(const IDBError& error)
     m_currentlyCompletingRequest = nullptr;
 
     connectionProxy().forgetActiveOperations(operations);
+    connectionProxy().forgetTransaction(*this);
 
     m_pendingTransactionOperationQueue.clear();
     m_abortQueue.clear();
index 15ac07e..497772d 100644 (file)
@@ -517,6 +517,15 @@ void IDBConnectionProxy::forgetActiveOperations(const Vector<RefPtr<TransactionO
         m_activeOperations.remove(operation->identifier());
 }
 
+void IDBConnectionProxy::forgetTransaction(IDBTransaction& transaction)
+{
+    Locker<Lock> locker(m_transactionMapLock);
+
+    m_pendingTransactions.remove(transaction.info().identifier());
+    m_committingTransactions.remove(transaction.info().identifier());
+    m_abortingTransactions.remove(transaction.info().identifier());
+}
+
 template<typename KeyType, typename ValueType>
 void removeItemsMatchingCurrentThread(HashMap<KeyType, ValueType>& map)
 {
index 2d039bf..588c077 100644 (file)
@@ -122,6 +122,7 @@ public:
     void unregisterDatabaseConnection(IDBDatabase&);
 
     void forgetActiveOperations(const Vector<RefPtr<TransactionOperation>>&);
+    void forgetTransaction(IDBTransaction&);
     void forgetActivityForCurrentThread();
 
 private:
index 74eb736..a30abfb 100644 (file)
@@ -85,6 +85,9 @@ public:
     {
         ASSERT(m_originThread.ptr() == &Thread::current());
 
+        if (m_performFunction)
+            m_performFunction = { };
+
         // Due to race conditions between the server sending an "operation complete" message and the client
         // forcefully aborting an operation, it's unavoidable that this method might be called twice.
         // It's okay to handle that gracefully with an early return.
@@ -98,8 +101,6 @@ public:
         // so we need to do this trick to null it out without first destroying it.
         Function<void(const IDBResultData&)> oldCompleteFunction;
         std::swap(m_completeFunction, oldCompleteFunction);
-
-        m_performFunction = { };
     }
 
     const IDBResourceIdentifier& identifier() const { return m_identifier; }