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

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Brady Eidson]

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.

[Brady Eidson]

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]

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]

Created attachment 281192 [details]
Patch

[Alex Christensen]

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]

(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]

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

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

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

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]

(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]

Created attachment 281215 [details]
Patch

[Alex Christensen]

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 on attachment 281215 [details]
Patch

Clearing flags on attachment: 281215

Committed r202019: <http://trac.webkit.org/changeset/202019>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.