Skip to content
Permalink
Browse files
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.


Canonical link: https://commits.webkit.org/209367@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@242043 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
szewai committed Feb 25, 2019
1 parent 7f311b3 commit 7a274f36f33c0ebaa4d09d08ef63b265b97227a2
Showing 12 changed files with 115 additions and 2 deletions.
@@ -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
@@ -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().
@@ -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 ]

@@ -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

@@ -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>
@@ -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.
@@ -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);
@@ -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();
}

@@ -152,6 +152,8 @@ class IDBTransaction : public ThreadSafeRefCounted<IDBTransaction>, public Event

void visitReferencedObjectStores(JSC::SlotVisitor&) const;

WEBCORE_EXPORT static std::atomic<unsigned> numberOfIDBTransactions;

private:
IDBTransaction(IDBDatabase&, const IDBTransactionInfo&, IDBOpenDBRequest*);

@@ -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;
@@ -378,6 +378,8 @@ class Internals final : public RefCounted<Internals>, private ContextDestruction
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;
@@ -405,6 +405,8 @@ enum CompositingPolicy {
void beginSimulatedMemoryPressure();
void endSimulatedMemoryPressure();

unsigned long numberOfIDBTransactions();

unsigned long numberOfLiveNodes();
unsigned long numberOfLiveDocuments();
unsigned long referencingNodeCount(Document document);

0 comments on commit 7a274f3

Please sign in to comment.