Bug 98078 – IndexedDB: IDBRequest/IDBCursor reference cycle not broken until transaction completes

• Status: RESOLVED
• Resolution: CONFIGURATION CHANGED
• Component: WebCore Misc.

[Joshua Bell]

IDBRequest maintains a RefPtr to IDBCursor (per spec, as |result|)
IDBCursor maintains a RefPtr to IDBRequest (per spec, as calling continue() re-uses the same cursor object)

We break this reference explicitly if (1) the transaction completes (via IDBTransaction::closeOpenCursors() => IDBCursor::close()) or (2) if the cursor hits the end (via IDBRequest::onSuccessInternal => IDBCursor::close()). 

But until that time, the cursor holds on to values which can eat up memory on both the front and and back end. If script has released all references to both the cursor and request we should be able to destroy them and reclaim memory.

As a workaround, scripts can call continue() with a value past the end of the range, e.g. cursor.continue(-Infinity) or cursor.continue([[[]]]), depending on key structure.

[Joshua Bell]

Created attachment 166919 [details]
Repro case for cursor issue

Attachd repro case c/o [e-mail] from crbug.com/145982
A reproduction case is attached.  findversion_nestedincomplete produces the problem, and findversion_testfix is the same code patched to always exhaust the cursors.  _tab.html is the code running in a tab, _sw.html is the same running in a shared worker.  Run the _tab.html version first to allow the database to be established and populated.  Runs after that reuse the database and should be good for profiling purposes.

I could be wrong, but it didn't seem to me that the issue resolves itself after the transaction completes.  I was seeing persistently high memory use ongoing.  If I made NUM_DOCUMENTS and/or VALUE_SIZE large enough in these repro cases I could also cause reliable crashes while preforming the read transaction.  I would have thought memory use by a scan would be related to the max size of an object in the store, rather than its total size.

[Joshua Bell]

Outline for a possible fix:

1. Have IDBRequest and IDBCursor derive from RefCountedBase instead
2. Override deref() on both to call RefCountedBase::deref(), then into a new IDBRequest::maybeBreakRefCycle(); if the cursor doesn't have a request ref, this is moot
3. IDBRequest::maybeBreakRefCycle() checks IDBRequest::isReferencingCursor() and IDBCursor::isReferencingRequest(); if false, this is moot.
4. IDBRequest::maybeBreakRefCycle() checks the refcounts. If both are 1, the request derefs the cursor.

[Joshua Bell]

I've copied this to blink. The issue is still valid for WebKit. I'm pinging possible new owners.

[Sihui Liu]

We have made related changes since the bug was filed and we have layout tests to ensure no ref cycle between IDBCursor and IDBRequest:
LayoutTests/storage/indexeddb/cursor-request-cycle.html

Closing this.