Changeset 242043 in webkit

Timestamp:

Feb 25, 2019 8:50:20 AM (5 years ago)

Author:

sihui_liu@apple.com

Message:

IndexedDB: IDBDatabase and IDBTransaction are leaked in layout tests
​https://bugs.webkit.org/show_bug.cgi?id=194709

Reviewed by Geoffrey Garen.

Source/WebCore:

When connection to IDB server is closed, IDBTransaction would abort without notifying IDBDatabase, so
IDBDatabase didn't clear its reference to IDBTransaction which created a reference cycle.

Also IDBTransaction didn't clear its reference to IDBRequest in this case and it led to another reference cycle
between IDBOpenDBRequest and IDBTransaction.

Test: storage/indexeddb/IDBObject-leak.html

• Modules/indexeddb/IDBDatabase.cpp:

(WebCore::IDBDatabase::connectionToServerLost):

• Modules/indexeddb/IDBTransaction.cpp:

(WebCore::IDBTransaction::IDBTransaction):
(WebCore::IDBTransaction::~IDBTransaction):
(WebCore::IDBTransaction::connectionClosedFromServer):

• Modules/indexeddb/IDBTransaction.h:
• testing/Internals.cpp:

(WebCore::Internals::numberOfIDBTransactions const):

• testing/Internals.h:
• testing/Internals.idl:

LayoutTests:

• TestExpectations:
• platform/wk2/TestExpectations:
• storage/indexeddb/IDBObject-leak-expected.txt: Added.
• storage/indexeddb/IDBObject-leak.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/TestExpectations (modified) (1 diff)
• LayoutTests/platform/wk2/TestExpectations (modified) (1 diff)
• LayoutTests/storage/indexeddb/IDBObject-leak-expected.txt (added)
• LayoutTests/storage/indexeddb/IDBObject-leak.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/Modules/indexeddb/IDBDatabase.cpp (modified) (1 diff)
• Source/WebCore/Modules/indexeddb/IDBTransaction.cpp (modified) (6 diffs)
• Source/WebCore/Modules/indexeddb/IDBTransaction.h (modified) (1 diff)
• Source/WebCore/testing/Internals.cpp (modified) (2 diffs)
• Source/WebCore/testing/Internals.h (modified) (1 diff)
• Source/WebCore/testing/Internals.idl (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2019-02-25  Sihui Liu  <sihui_liu@apple.com>
+
+        IndexedDB: IDBDatabase and IDBTransaction are leaked in layout tests
+        https://bugs.webkit.org/show_bug.cgi?id=194709
+
+        Reviewed by Geoffrey Garen.
+
+        * TestExpectations:
+        * platform/wk2/TestExpectations:
+        * storage/indexeddb/IDBObject-leak-expected.txt: Added.
+        * storage/indexeddb/IDBObject-leak.html: Added.
+
 2019-02-25  Zan Dobersek  <zdobersek@igalia.com>
 

trunk/LayoutTests/TestExpectations

@@ -131 +131 @@
 http/tests/navigation/useragent-reload.php [ Skip ]
 storage/indexeddb/modern/opendatabase-after-storage-crash.html [ Skip ]
+storage/indexeddb/IDBObject-leak.html [ Skip ]
 fast/forms/call-text-did-change-in-text-field-when-typing.html [ Skip ]
 

trunk/LayoutTests/platform/wk2/TestExpectations

@@ -747 +747 @@
 http/tests/navigation/useragent-reload.php [ Pass ]
 storage/indexeddb/modern/opendatabase-after-storage-crash.html [ Pass ]
+storage/indexeddb/IDBObject-leak.html [ Pass ]
 
 fast/forms/call-text-did-change-in-text-field-when-typing.html [ Pass ]

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2019-02-25  Sihui Liu  <sihui_liu@apple.com>
+
+        IndexedDB: IDBDatabase and IDBTransaction are leaked in layout tests
+        https://bugs.webkit.org/show_bug.cgi?id=194709
+
+        Reviewed by Geoffrey Garen.
+
+        When connection to IDB server is closed, IDBTransaction would abort without notifying IDBDatabase, so 
+        IDBDatabase didn't clear its reference to IDBTransaction which created a reference cycle. 
+ 
+        Also IDBTransaction didn't clear its reference to IDBRequest in this case and it led to another reference cycle
+        between IDBOpenDBRequest and IDBTransaction.
+
+        Test: storage/indexeddb/IDBObject-leak.html
+
+        * Modules/indexeddb/IDBDatabase.cpp:
+        (WebCore::IDBDatabase::connectionToServerLost):
+        * Modules/indexeddb/IDBTransaction.cpp:
+        (WebCore::IDBTransaction::IDBTransaction):
+        (WebCore::IDBTransaction::~IDBTransaction):
+        (WebCore::IDBTransaction::connectionClosedFromServer):
+        * Modules/indexeddb/IDBTransaction.h:
+        * testing/Internals.cpp:
+        (WebCore::Internals::numberOfIDBTransactions const):
+        * testing/Internals.h:
+        * testing/Internals.idl:
+
 2019-02-25  Zalan Bujtas  <zalan@apple.com>
 

trunk/Source/WebCore/Modules/indexeddb/IDBDatabase.cpp

@@ -265 +265 @@
     m_closedInServer = true;
 
-    for (auto& transaction : m_activeTransactions.values())
+    auto transactions = copyToVector(m_activeTransactions.values());
+    for (auto& transaction : transactions)
         transaction->connectionClosedFromServer(error);
 

trunk/Source/WebCore/Modules/indexeddb/IDBTransaction.cpp

@@ -60 +60 @@
 using namespace JSC;
 
+std::atomic<unsigned> IDBTransaction::numberOfIDBTransactions { 0 };
+
 Ref<IDBTransaction> IDBTransaction::create(IDBDatabase& database, const IDBTransactionInfo& info)
 {
@@ -83 +85 @@
     ASSERT(&m_database->originThread() == &Thread::current());
 
+    ++numberOfIDBTransactions;
+
     if (m_info.mode() == IDBTransactionMode::Versionchange) {
         ASSERT(m_openDBRequest);
@@ -106 +110 @@
 IDBTransaction::~IDBTransaction()
 {
+    --numberOfIDBTransactions;
     ASSERT(&m_database->originThread() == &Thread::current());
 }
@@ -1435 +1440 @@
     LOG(IndexedDB, "IDBTransaction::connectionClosedFromServer - %s", error.message().utf8().data());
 
-    m_state = IndexedDB::TransactionState::Aborting;
+    m_database->willAbortTransaction(*this);
+    transitionedToFinishing(IndexedDB::TransactionState::Aborting);
 
     abortInProgressOperations(error);
@@ -1446 +1452 @@
         operation->doComplete(IDBResultData::error(operation->identifier(), error));
     }
+    m_currentlyCompletingRequest = nullptr;
 
     connectionProxy().forgetActiveOperations(operations);
@@ -1455 +1462 @@
     m_idbError = error;
     m_domError = error.toDOMException();
+    m_database->didAbortTransaction(*this);
     fireOnAbort();
 }

trunk/Source/WebCore/Modules/indexeddb/IDBTransaction.h

@@ -153 +153 @@
     void visitReferencedObjectStores(JSC::SlotVisitor&) const;
 
+    WEBCORE_EXPORT static std::atomic<unsigned> numberOfIDBTransactions;
+
 private:
     IDBTransaction(IDBDatabase&, const IDBTransactionInfo&, IDBOpenDBRequest*);

trunk/Source/WebCore/testing/Internals.cpp

@@ -94 +94 @@
 #include "HistoryItem.h"
 #include "HitTestResult.h"
+#include "IDBRequest.h"
+#include "IDBTransaction.h"
 #include "InspectorClient.h"
 #include "InspectorController.h"
@@ -2375 +2377 @@
 }
 
+unsigned Internals::numberOfIDBTransactions() const
+{
+    return IDBTransaction::numberOfIDBTransactions;
+}
+
 unsigned Internals::numberOfLiveNodes() const
 {

trunk/Source/WebCore/testing/Internals.h

@@ -379 +379 @@
     ExceptionOr<void> insertUserCSS(const String&) const;
 
+    unsigned numberOfIDBTransactions() const;
+
     unsigned numberOfLiveNodes() const;
     unsigned numberOfLiveDocuments() const;

trunk/Source/WebCore/testing/Internals.idl

@@ -406 +406 @@
     void endSimulatedMemoryPressure();
 
+    unsigned long numberOfIDBTransactions();
+
     unsigned long numberOfLiveNodes();
     unsigned long numberOfLiveDocuments();