Bug 158643 - storage/indexeddb/modern/leaks-1.html leaks the database connection handle
Summary: storage/indexeddb/modern/leaks-1.html leaks the database connection handle
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Brady Eidson
URL:
Keywords:
Depends on:
Blocks: 154015
  Show dependency treegraph
 
Reported: 2016-06-10 16:57 PDT by Brady Eidson
Modified: 2016-06-13 17:32 PDT (History)
9 users (show)

See Also:


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

Note You need to log in before you can comment on or make changes to this bug.
Description Brady Eidson 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.
Comment 1 Brady Eidson 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.
Comment 2 Brady Eidson 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.
Comment 3 Brady Eidson 2016-06-13 13:10:22 PDT
Created attachment 281192 [details]
Patch
Comment 4 Alex Christensen 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.
Comment 5 Brady Eidson 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
Comment 6 Brady Eidson 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.
Comment 7 Build Bot 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
Comment 8 Build Bot 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
Comment 9 Build Bot 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
Comment 10 Build Bot 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
Comment 11 Build Bot 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
Comment 12 Build Bot 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
Comment 13 Brady Eidson 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.
Comment 14 Brady Eidson 2016-06-13 16:45:21 PDT
Created attachment 281215 [details]
Patch
Comment 15 Alex Christensen 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.
Comment 16 WebKit Commit Bot 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>
Comment 17 WebKit Commit Bot 2016-06-13 17:32:41 PDT
All reviewed patches have been landed.  Closing bug.