Bug 158643

Summary:

storage/indexeddb/modern/leaks-1.html leaks the database connection handle

Product:

WebKit

Reporter:

Brady Eidson <beidson>

Component:

WebCore Misc.

Assignee:

Brady Eidson <beidson>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

achristensen, alecflett, buildbot, cdumez, commit-queue, esprehn+autocc, jsbell, kangil.han, rniwa

Priority:

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

154015

Attachments:

Description	Flags
Patch	none
Archive of layout-test-results from ews101 for mac-yosemite	none
Archive of layout-test-results from ews105 for mac-yosemite-wk2	none
Archive of layout-test-results from ews114 for mac-yosemite	none
Patch	none

Brady Eidson

Reported 2016-06-10 16:57:56 PDT

storage/indexeddb/modern/leaks-1.html leaks the database connection handle By the time the test is over two objects retain their ref on the database: 1 - Its IDBOpenDBRequest 2 - The IDBTransaction representing the version change transaction.

Attachments
Patch (3.82 KB, patch) 2016-06-13 13:10 PDT, Brady Eidson	no flags	Details Formatted Diff Diff
Archive of layout-test-results from ews101 for mac-yosemite (830.18 KB, application/zip) 2016-06-13 13:59 PDT, Build Bot	no flags	Details
Archive of layout-test-results from ews105 for mac-yosemite-wk2 (814.40 KB, application/zip) 2016-06-13 14:02 PDT, Build Bot	no flags	Details
Archive of layout-test-results from ews114 for mac-yosemite (1.56 MB, application/zip) 2016-06-13 14:18 PDT, Build Bot	no flags	Details
Patch (6.43 KB, patch) 2016-06-13 16:45 PDT, Brady Eidson	no flags	Details Formatted Diff Diff
Show Obsolete (4) View All Add attachment proposed patch, testcase, etc.

Brady Eidson

Comment 1 2016-06-10 17:18:44 PDT

Note that the wrapper can eventually get GC'ed in the test, but the impl object itself still has a couple of refs because of the above.

Brady Eidson

Comment 2 2016-06-13 13:07:21 PDT

After fixing an opendbrequest leak - http://trac.webkit.org/changeset/201995 - this is just down to hasPendingActivity being wrong. Patch coming up for that.

Brady Eidson

Comment 3 2016-06-13 13:10:22 PDT

Created attachment 281192 [details] Patch

Alex Christensen

Comment 4 2016-06-13 13:18:38 PDT

Comment on attachment 281192 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=281192&action=review > Source/WebCore/Modules/indexeddb/IDBDatabase.cpp:73 > + if (m_closedInServer) > + return false; Assert that all the transactions are empty.

Brady Eidson

Comment 5 2016-06-13 13:23:04 PDT

(In reply to comment #4) > Comment on attachment 281192 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=281192&action=review > > > Source/WebCore/Modules/indexeddb/IDBDatabase.cpp:73 > > + if (m_closedInServer) > > + return false; > > Assert that all the transactions are empty. Despite my off-the-cuff proclamation that we could do this, we can't, so NM

Brady Eidson

Comment 6 2016-06-13 13:51:47 PDT

Current EWS run attempts show IDB test failures including - OF COURSE - the very test included in the patch! Yay! Will try to repro locally.

Build Bot

Comment 7 2016-06-13 13:59:14 PDT

Comment on attachment 281192 [details] Patch Attachment 281192 [details] did not pass mac-ews (mac): Output: http://webkit-queues.webkit.org/results/1496322 New failing tests: storage/indexeddb/database-wrapper-private.html storage/indexeddb/modern/leak-1.html storage/indexeddb/database-wrapper.html

Build Bot

Comment 8 2016-06-13 13:59:16 PDT

Created attachment 281198 [details] Archive of layout-test-results from ews101 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-ews. Bot: ews101 Port: mac-yosemite Platform: Mac OS X 10.10.5

Build Bot

Comment 9 2016-06-13 14:02:49 PDT

Comment on attachment 281192 [details] Patch Attachment 281192 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-queues.webkit.org/results/1496325 New failing tests: storage/indexeddb/database-wrapper-private.html storage/indexeddb/modern/leak-1.html storage/indexeddb/database-wrapper.html

Build Bot

Comment 10 2016-06-13 14:02:52 PDT

Created attachment 281200 [details] Archive of layout-test-results from ews105 for mac-yosemite-wk2 The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews. Bot: ews105 Port: mac-yosemite-wk2 Platform: Mac OS X 10.10.5

Build Bot

Comment 11 2016-06-13 14:18:34 PDT

Comment on attachment 281192 [details] Patch Attachment 281192 [details] did not pass mac-debug-ews (mac): Output: http://webkit-queues.webkit.org/results/1496349 New failing tests: storage/indexeddb/database-wrapper-private.html storage/indexeddb/modern/leak-1.html storage/indexeddb/database-wrapper.html

Build Bot

Comment 12 2016-06-13 14:18:37 PDT

Created attachment 281202 [details] Archive of layout-test-results from ews114 for mac-yosemite The attached test failures were seen while running run-webkit-tests on the mac-debug-ews. Bot: ews114 Port: mac-yosemite Platform: Mac OS X 10.10.5

Brady Eidson

Comment 13 2016-06-13 14:35:23 PDT

(In reply to comment #11) > Comment on attachment 281192 [details] > Patch > > Attachment 281192 [details] did not pass mac-debug-ews (mac): > Output: http://webkit-queues.webkit.org/results/1496349 > > New failing tests: > storage/indexeddb/database-wrapper-private.html > storage/indexeddb/modern/leak-1.html > storage/indexeddb/database-wrapper.html Yup - I forgot to include updated test results for leak-1, and also hadn't seen the database-wrapper issues locally yet. They're easy to repro, and I'm exploring.

Brady Eidson

Comment 14 2016-06-13 16:45:21 PDT

Created attachment 281215 [details] Patch

Alex Christensen

Comment 15 2016-06-13 16:50:24 PDT

Comment on attachment 281215 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=281215&action=review > Source/WebCore/dom/EventTarget.h:181 > + return const_cast<EventTarget*>(this)->eventTargetData(); I think const_cast is always bad, but this case just enables us to call hasEventListeners, which doesn't modify anything. We should probably look into getting rid of this.

WebKit Commit Bot

Comment 16 2016-06-13 17:32:36 PDT

Comment on attachment 281215 [details] Patch Clearing flags on attachment: 281215 Committed r202019: <http://trac.webkit.org/changeset/202019>

WebKit Commit Bot

Comment 17 2016-06-13 17:32:41 PDT

All reviewed patches have been landed. Closing bug.

Note You need to log in before you can comment on or make changes to this bug.