IndexedDB: IDBDatabase and IDBTransaction are leaked in layout tests
authorsihui_liu@apple.com <sihui_liu@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 25 Feb 2019 16:50:20 +0000 (16:50 +0000)
committersihui_liu@apple.com <sihui_liu@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 25 Feb 2019 16:50:20 +0000 (16:50 +0000)
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.

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

12 files changed:
LayoutTests/ChangeLog
LayoutTests/TestExpectations
LayoutTests/platform/wk2/TestExpectations
LayoutTests/storage/indexeddb/IDBObject-leak-expected.txt [new file with mode: 0644]
LayoutTests/storage/indexeddb/IDBObject-leak.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/Modules/indexeddb/IDBDatabase.cpp
Source/WebCore/Modules/indexeddb/IDBTransaction.cpp
Source/WebCore/Modules/indexeddb/IDBTransaction.h
Source/WebCore/testing/Internals.cpp
Source/WebCore/testing/Internals.h
Source/WebCore/testing/Internals.idl

index a8020e6..a86be06 100644 (file)
@@ -1,3 +1,15 @@
+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>
 
         Unreviewed WPE gardening. Adding a few failure expectations as well
index 29817de..ada9652 100644 (file)
@@ -130,6 +130,7 @@ http/tests/storageAccess/ [ Skip ]
 http/tests/navigation/process-swap-window-open.html [ Skip ]
 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 ]
 
 # Only Mac and iOS have an implementation of UIScriptController::doAsyncTask().
index 42d447e..c40ff44 100644 (file)
@@ -746,6 +746,7 @@ http/wpt/cross-origin-resource-policy/ [ Pass ]
 
 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 ]
 
diff --git a/LayoutTests/storage/indexeddb/IDBObject-leak-expected.txt b/LayoutTests/storage/indexeddb/IDBObject-leak-expected.txt
new file mode 100644 (file)
index 0000000..b286595
--- /dev/null
@@ -0,0 +1,10 @@
+This test verifies that IDBTransaction objects are freed.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS internals.numberOfIDBTransactions() is 0
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/storage/indexeddb/IDBObject-leak.html b/LayoutTests/storage/indexeddb/IDBObject-leak.html
new file mode 100644 (file)
index 0000000..e18b68b
--- /dev/null
@@ -0,0 +1,40 @@
+<!DOCTYPE html>
+<script src="../../resources/js-test.js"></script>
+<script src="resources/shared.js"></script>
+<script>
+description('This test verifies that IDBTransaction objects are freed.');
+
+jsTestIsAsync = true;
+
+function test() {
+    if (!window.internals || !internals.numberOfIDBTransactions) {
+        testFailed('This test requires access to the Internals object');
+        finishJSTest();
+        return;
+    }
+
+    if (sessionStorage.doneFirstLoad) {
+        gc();
+        shouldBeEqualToNumber("internals.numberOfIDBTransactions()", 0);
+        finishJSTest();
+        return;
+    }
+
+    var dbname = setDBNameFromPath() + Date();
+    var request =  window.indexedDB.open(dbname);
+    request.onupgradeneeded = function(evt) {
+        sessionStorage.doneFirstLoad = true;
+        if (!window.testRunner || !testRunner.terminateNetworkProcess) {
+            testFailed('This test requires access to the TestRunner object and terminateNetworkProcess() function');
+            finishJSTest();
+            return;
+        } 
+        testRunner.terminateNetworkProcess();
+        setTimeout((()=> {
+            location.reload();
+        }), 0);
+    }
+}
+
+test();
+</script>
\ No newline at end of file
index 92a0d48..da62b6a 100644 (file)
@@ -1,3 +1,30 @@
+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>
 
         Add missing stream parameter. Unreviewed.
index 1847439..0a09572 100644 (file)
@@ -264,7 +264,8 @@ void IDBDatabase::connectionToServerLost(const IDBError& error)
     m_closePending = true;
     m_closedInServer = true;
 
-    for (auto& transaction : m_activeTransactions.values())
+    auto transactions = copyToVector(m_activeTransactions.values());
+    for (auto& transaction : transactions)
         transaction->connectionClosedFromServer(error);
 
     auto errorEvent = Event::create(m_eventNames.errorEvent, Event::CanBubble::Yes, Event::IsCancelable::No);
index 2e965e9..a3995d3 100644 (file)
@@ -59,6 +59,8 @@
 namespace WebCore {
 using namespace JSC;
 
+std::atomic<unsigned> IDBTransaction::numberOfIDBTransactions { 0 };
+
 Ref<IDBTransaction> IDBTransaction::create(IDBDatabase& database, const IDBTransactionInfo& info)
 {
     return adoptRef(*new IDBTransaction(database, info, nullptr));
@@ -82,6 +84,8 @@ IDBTransaction::IDBTransaction(IDBDatabase& database, const IDBTransactionInfo&
     LOG(IndexedDB, "IDBTransaction::IDBTransaction - %s", m_info.loggingString().utf8().data());
     ASSERT(&m_database->originThread() == &Thread::current());
 
+    ++numberOfIDBTransactions;
+
     if (m_info.mode() == IDBTransactionMode::Versionchange) {
         ASSERT(m_openDBRequest);
         m_openDBRequest->setVersionChangeTransaction(*this);
@@ -105,6 +109,7 @@ IDBTransaction::IDBTransaction(IDBDatabase& database, const IDBTransactionInfo&
 
 IDBTransaction::~IDBTransaction()
 {
+    --numberOfIDBTransactions;
     ASSERT(&m_database->originThread() == &Thread::current());
 }
 
@@ -1434,7 +1439,8 @@ void IDBTransaction::connectionClosedFromServer(const IDBError& error)
 {
     LOG(IndexedDB, "IDBTransaction::connectionClosedFromServer - %s", error.message().utf8().data());
 
-    m_state = IndexedDB::TransactionState::Aborting;
+    m_database->willAbortTransaction(*this);
+    transitionedToFinishing(IndexedDB::TransactionState::Aborting);
 
     abortInProgressOperations(error);
 
@@ -1445,6 +1451,7 @@ void IDBTransaction::connectionClosedFromServer(const IDBError& error)
         ASSERT(m_transactionOperationsInProgressQueue.first() == operation.get());
         operation->doComplete(IDBResultData::error(operation->identifier(), error));
     }
+    m_currentlyCompletingRequest = nullptr;
 
     connectionProxy().forgetActiveOperations(operations);
 
@@ -1454,6 +1461,7 @@ void IDBTransaction::connectionClosedFromServer(const IDBError& error)
 
     m_idbError = error;
     m_domError = error.toDOMException();
+    m_database->didAbortTransaction(*this);
     fireOnAbort();
 }
 
index 722f12c..6a2815f 100644 (file)
@@ -152,6 +152,8 @@ public:
 
     void visitReferencedObjectStores(JSC::SlotVisitor&) const;
 
+    WEBCORE_EXPORT static std::atomic<unsigned> numberOfIDBTransactions;
+
 private:
     IDBTransaction(IDBDatabase&, const IDBTransactionInfo&, IDBOpenDBRequest*);
 
index 77a6071..cf8a685 100644 (file)
@@ -93,6 +93,8 @@
 #include "HistoryController.h"
 #include "HistoryItem.h"
 #include "HitTestResult.h"
+#include "IDBRequest.h"
+#include "IDBTransaction.h"
 #include "InspectorClient.h"
 #include "InspectorController.h"
 #include "InspectorFrontendClientLocal.h"
@@ -2374,6 +2376,11 @@ ExceptionOr<unsigned> Internals::countFindMatches(const String& text, const Vect
     return document->page()->countFindMatches(text, parsedOptions.releaseReturnValue(), 1000);
 }
 
+unsigned Internals::numberOfIDBTransactions() const
+{
+    return IDBTransaction::numberOfIDBTransactions;
+}
+
 unsigned Internals::numberOfLiveNodes() const
 {
     unsigned nodeCount = 0;
index da3fe1d..48ecfc8 100644 (file)
@@ -378,6 +378,8 @@ public:
     ExceptionOr<void> insertAuthorCSS(const String&) const;
     ExceptionOr<void> insertUserCSS(const String&) const;
 
+    unsigned numberOfIDBTransactions() const;
+
     unsigned numberOfLiveNodes() const;
     unsigned numberOfLiveDocuments() const;
     unsigned referencingNodeCount(const Document&) const;
index cb7d2a4..f7935ce 100644 (file)
@@ -405,6 +405,8 @@ enum CompositingPolicy {
     void beginSimulatedMemoryPressure();
     void endSimulatedMemoryPressure();
 
+    unsigned long numberOfIDBTransactions();
+
     unsigned long numberOfLiveNodes();
     unsigned long numberOfLiveDocuments();
     unsigned long referencingNodeCount(Document document);