Modern IDB: Ref cycle between IDBObjectStore and IDBTransaction

• Component: WebCore Misc.
• Status: RESOLVED FIXED

[Brady Eidson]

Modern IDB: Ref cycle between IDBObjectStore and IDBTransaction

IDBTransaction has to retain a list of all object stores referenced during its lifetime, and object stores need a pointer to their transaction.

Fortunately it's easy to break this cycle: Once the transaction is aborting or committing, accessing its referenced object stores is no longer possible.

So at that time, the set of references object stores can be cleared, breaking the cycle.

[Brady Eidson]

Created attachment 270969 [details]
Patch v1

[Build Bot]

Comment on attachment 270969 [details]
Patch v1

Attachment 270969 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/807768

New failing tests:
storage/indexeddb/objectstore-basics-private.html
storage/indexeddb/modern/abort-objectstore-info-private.html
storage/indexeddb/modern/abort-objectstore-info.html
storage/indexeddb/metadata.html
storage/indexeddb/objectstore-basics.html
storage/indexeddb/metadata-private.html

[Build Bot]

Created attachment 270974 [details]
Archive of layout-test-results from ews103 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Build Bot]

Comment on attachment 270969 [details]
Patch v1

Attachment 270969 [details] did not pass mac-debug-ews (mac):
Output: http://webkit-queues.webkit.org/results/807775

New failing tests:
storage/indexeddb/objectstore-basics-private.html
storage/indexeddb/modern/abort-objectstore-info-private.html
storage/indexeddb/modern/abort-objectstore-info.html
storage/indexeddb/metadata.html
storage/indexeddb/objectstore-basics.html
storage/indexeddb/metadata-private.html

[Build Bot]

Created attachment 270975 [details]
Archive of layout-test-results from ews113 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews113  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Brady Eidson]

(In reply to comment #4)
> Comment on attachment 270969 [details]
> Patch v1
> 
> Attachment 270969 [details] did not pass mac-debug-ews (mac):
> Output: http://webkit-queues.webkit.org/results/807775
> 
> New failing tests:
> storage/indexeddb/objectstore-basics-private.html
> storage/indexeddb/modern/abort-objectstore-info-private.html
> storage/indexeddb/modern/abort-objectstore-info.html
> storage/indexeddb/metadata.html
> storage/indexeddb/objectstore-basics.html
> storage/indexeddb/metadata-private.html

Very surprising, looking locally.

[Brady Eidson]

Easy bug. Building and testing now.

[Brady Eidson]

Created attachment 271004 [details]
Patch v2

[WebKit Commit Bot]

Comment on attachment 271004 [details]
Patch v2

Clearing flags on attachment: 271004

Committed r196373: <http://trac.webkit.org/changeset/196373>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Darin Adler]

Comment on attachment 271004 [details]
Patch v2

View in context: https://bugs.webkit.org/attachment.cgi?id=271004&action=review

> Source/WebCore/Modules/indexeddb/client/IDBTransactionImpl.cpp:183
> +    ASSERT(!isFinishedOrFinishing());
> +    m_state = state;

Can we add this:

    ASSERT(isFinishedOrFinishing());

after setting m_state?

[Brady Eidson]

(In reply to comment #11)
> Comment on attachment 271004 [details]
> Patch v2
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=271004&action=review
> 
> > Source/WebCore/Modules/indexeddb/client/IDBTransactionImpl.cpp:183
> > +    ASSERT(!isFinishedOrFinishing());
> > +    m_state = state;
> 
> Can we add this:
> 
>     ASSERT(isFinishedOrFinishing());
> 
> after setting m_state?

Not a bad idea!

[Brady Eidson]

Reopening to track that feedback so I don't forget it.

[Alex Christensen]

http://trac.webkit.org/changeset/196455