WebKit Bugzilla
Attachment 362504 Details for
Bug 194709
: IndexedDB: IDBDatabase and IDBTransaction are leaked in layout tests
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch
bug-194709-20190220093335.patch (text/plain), 11.74 KB, created by
Sihui Liu
on 2019-02-20 09:33:36 PST
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
Sihui Liu
Created:
2019-02-20 09:33:36 PST
Size:
11.74 KB
patch
obsolete
>Subversion Revision: 241761 >diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog >index a6da53fe8e04ad79dd06c117c410a040b2b97e76..5f2d6a1a72972fcbac7a1e97c60744884e7d2e65 100644 >--- a/Source/WebCore/ChangeLog >+++ b/Source/WebCore/ChangeLog >@@ -1,3 +1,30 @@ >+2019-02-20 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 NOBODY (OOPS!). >+ >+ 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-19 Commit Queue <commit-queue@webkit.org> > > Unreviewed, rolling out r241722. >diff --git a/Source/WebCore/Modules/indexeddb/IDBDatabase.cpp b/Source/WebCore/Modules/indexeddb/IDBDatabase.cpp >index 1847439190a8b79bef706432fd2f9d7556907365..0a095725fd2fbd324b957118924149eba26364e7 100644 >--- a/Source/WebCore/Modules/indexeddb/IDBDatabase.cpp >+++ b/Source/WebCore/Modules/indexeddb/IDBDatabase.cpp >@@ -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); >diff --git a/Source/WebCore/Modules/indexeddb/IDBTransaction.cpp b/Source/WebCore/Modules/indexeddb/IDBTransaction.cpp >index 2e965e97d2909ec0c70f3f8741a188ff6c549b2f..d384bd3316de2540839ea6d1910a2185bde01b4a 100644 >--- a/Source/WebCore/Modules/indexeddb/IDBTransaction.cpp >+++ b/Source/WebCore/Modules/indexeddb/IDBTransaction.cpp >@@ -81,6 +81,13 @@ IDBTransaction::IDBTransaction(IDBDatabase& database, const IDBTransactionInfo& > { > LOG(IndexedDB, "IDBTransaction::IDBTransaction - %s", m_info.loggingString().utf8().data()); > ASSERT(&m_database->originThread() == &Thread::current()); >+ >+ { >+ LockHolder locker(m_countMutex); >+ auto addResult = allIDBTransactions().add(this); >+ ASSERT_UNUSED(addResult, addResult.isNewEntry); >+ } >+ > > if (m_info.mode() == IDBTransactionMode::Versionchange) { > ASSERT(m_openDBRequest); >@@ -105,9 +112,20 @@ IDBTransaction::IDBTransaction(IDBDatabase& database, const IDBTransactionInfo& > > IDBTransaction::~IDBTransaction() > { >+ { >+ LockHolder locker(m_countMutex); >+ ASSERT(allIDBTransactions().contains(this)); >+ allIDBTransactions().remove(this); >+ } > ASSERT(&m_database->originThread() == &Thread::current()); > } > >+HashSet<IDBTransaction*>& IDBTransaction::allIDBTransactions() >+{ >+ static NeverDestroyed<HashSet<IDBTransaction*>> transactions; >+ return transactions; >+} >+ > IDBClient::IDBConnectionProxy& IDBTransaction::connectionProxy() > { > return m_database->connectionProxy(); >@@ -1434,7 +1452,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 +1464,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 +1474,7 @@ void IDBTransaction::connectionClosedFromServer(const IDBError& error) > > m_idbError = error; > m_domError = error.toDOMException(); >+ m_database->didAbortTransaction(*this); > fireOnAbort(); > } > >diff --git a/Source/WebCore/Modules/indexeddb/IDBTransaction.h b/Source/WebCore/Modules/indexeddb/IDBTransaction.h >index 722f12c84a787dc1a32a33bc5d236f0cecdc4f04..7ac79ae599be68e6e3b09105526cd73650057c8b 100644 >--- a/Source/WebCore/Modules/indexeddb/IDBTransaction.h >+++ b/Source/WebCore/Modules/indexeddb/IDBTransaction.h >@@ -152,6 +152,8 @@ public: > > void visitReferencedObjectStores(JSC::SlotVisitor&) const; > >+ WEBCORE_EXPORT static HashSet<IDBTransaction*>& allIDBTransactions(); >+ > private: > IDBTransaction(IDBDatabase&, const IDBTransactionInfo&, IDBOpenDBRequest*); > >@@ -258,6 +260,8 @@ private: > RefPtr<IDBRequest> m_currentlyCompletingRequest; > > bool m_contextStopped { false }; >+ >+ Lock m_countMutex; > }; > > class TransactionActivator { >diff --git a/Source/WebCore/testing/Internals.cpp b/Source/WebCore/testing/Internals.cpp >index b06a1f83c37c84b6271d09d29a2532bd77d4cc60..c30d5e1dd29c7a93100e7ec37939715c9e1c4c4b 100644 >--- a/Source/WebCore/testing/Internals.cpp >+++ b/Source/WebCore/testing/Internals.cpp >@@ -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" >@@ -2383,6 +2385,11 @@ ExceptionOr<unsigned> Internals::countFindMatches(const String& text, const Vect > return document->page()->countFindMatches(text, parsedOptions.releaseReturnValue(), 1000); > } > >+unsigned Internals::numberOfIDBTransactions() const >+{ >+ return IDBTransaction::allIDBTransactions().size(); >+} >+ > unsigned Internals::numberOfLiveNodes() const > { > unsigned nodeCount = 0; >diff --git a/Source/WebCore/testing/Internals.h b/Source/WebCore/testing/Internals.h >index c19382f1639c5dadff3dd36bab16d71a2369ecb3..4e4d9d74a774658c8d086c6ec6c8341a07b86bd2 100644 >--- a/Source/WebCore/testing/Internals.h >+++ b/Source/WebCore/testing/Internals.h >@@ -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; >diff --git a/Source/WebCore/testing/Internals.idl b/Source/WebCore/testing/Internals.idl >index e638512de819c0c105b094b3549fa51cb0982e33..01399ec80ade258a1a19dfd3c6ff6db7697ef8ed 100644 >--- a/Source/WebCore/testing/Internals.idl >+++ b/Source/WebCore/testing/Internals.idl >@@ -405,6 +405,8 @@ enum CompositingPolicy { > void beginSimulatedMemoryPressure(); > void endSimulatedMemoryPressure(); > >+ unsigned long numberOfIDBTransactions(); >+ > unsigned long numberOfLiveNodes(); > unsigned long numberOfLiveDocuments(); > unsigned long referencingNodeCount(Document document); >diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog >index e26eaf6e984904ac4398e158332c2c689e209500..bc262c8e95bf7aa0114668a458f9a9e6349c5a3e 100644 >--- a/LayoutTests/ChangeLog >+++ b/LayoutTests/ChangeLog >@@ -1,3 +1,15 @@ >+2019-02-20 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 NOBODY (OOPS!). >+ >+ * TestExpectations: >+ * platform/wk2/TestExpectations: >+ * storage/indexeddb/IDBObject-leak-expected.txt: Added. >+ * storage/indexeddb/IDBObject-leak.html: Added. >+ > 2019-02-19 Commit Queue <commit-queue@webkit.org> > > Unreviewed, rolling out r241722. >diff --git a/LayoutTests/TestExpectations b/LayoutTests/TestExpectations >index c2c19fb1153ea659530df86108bd5fff7d9cb26a..b7c46238f0d531d73abebcba5aae5d24e94f1d47 100644 >--- a/LayoutTests/TestExpectations >+++ b/LayoutTests/TestExpectations >@@ -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(). >diff --git a/LayoutTests/platform/wk2/TestExpectations b/LayoutTests/platform/wk2/TestExpectations >index 42d447e536b1573e903040a69a385f793a48400a..c40ff4402a061abaf6a840722556e7256f766e17 100644 >--- a/LayoutTests/platform/wk2/TestExpectations >+++ b/LayoutTests/platform/wk2/TestExpectations >@@ -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 >index 0000000000000000000000000000000000000000..b286595a5bdb0f7b0982880b1f9f3170420539c8 >--- /dev/null >+++ b/LayoutTests/storage/indexeddb/IDBObject-leak-expected.txt >@@ -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 >index 0000000000000000000000000000000000000000..742838da689c235545c2ce1757cefe8fe57a5506 >--- /dev/null >+++ b/LayoutTests/storage/indexeddb/IDBObject-leak.html >@@ -0,0 +1,39 @@ >+<!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.'); >+ >+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.waitUntilDone(); >+ testRunner.terminateNetworkProcess(); >+ setTimeout((()=> { >+ location.reload(); >+ }), 1); >+ } >+} >+ >+test(); >+</script> >\ No newline at end of file
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 194709
:
362122
|
362196
|
362300
|
362332
|
362346
|
362353
|
362356
|
362504
|
362509
|
362516
|
362517
|
362773