Bug 154015

I'll be exploring this bit by bit to come up with the best lifetime strategy and logging my findings here. First finding that directs future exploration: On a very basic test page that: -opens a new database -creates one object store -lets the transaction finish -navigates away to a new page ...The IDBOpenDBRequest remains alive with a ref-count of 1. The holder of that ref is the IDBTransaction that represents the version change transaction. Additionally, there's a circular reference between the IDBOpenDBRequest and the IDBTransaction. The transaction doesn't need the request anymore after the last event has fired, so that ref should be broken, which would break the ref on the transaction itself.

Next in line is IDBDatabase: It has a couple of refs that come and go during event dispatch, all nicely balanced. It's initial ref is held as the result of the OpenDBRequest. So as long as the open DB request is cleaned up, we're fine. In the case of an upgrade needed, it's next ref is held by the version change IDBTransaction. And, in fact, that ref lasts as long as the transaction object, which is "a very very long time." To see if there's anything circular there, I'll now dig in to the transaction lifetime.

(In reply to comment #2) > Next in line is IDBDatabase: > > It has a couple of refs that come and go during event dispatch, all nicely > balanced. > > It's initial ref is held as the result of the OpenDBRequest. So as long as > the open DB request is cleaned up, we're fine. > > In the case of an upgrade needed, it's next ref is held by the version > change IDBTransaction. > > And, in fact, that ref lasts as long as the transaction object, which is "a > very very long time." > > To see if there's anything circular there, I'll now dig in to the > transaction lifetime. Transactions leak a couple of different ways. One huge way is that TransactionOperations leak! Yikes. That one is an easy fix. https://bugs.webkit.org/show_bug.cgi?id=154054 for that

(In reply to comment #3) > (In reply to comment #2) > > Next in line is IDBDatabase: > > > > It has a couple of refs that come and go during event dispatch, all nicely > > balanced. > > > > It's initial ref is held as the result of the OpenDBRequest. So as long as > > the open DB request is cleaned up, we're fine. > > > > In the case of an upgrade needed, it's next ref is held by the version > > change IDBTransaction. > > > > And, in fact, that ref lasts as long as the transaction object, which is "a > > very very long time." > > > > To see if there's anything circular there, I'll now dig in to the > > transaction lifetime. > > Transactions leak a couple of different ways. > > One huge way is that TransactionOperations leak! Yikes. That one is an easy > fix. > > https://bugs.webkit.org/show_bug.cgi?id=154054 for that So with the fix for 154054, and with the previous fix for IDBOpenDBRequests, the most basic example of IDB usage works without leaks. Attaching that "basic-test.html" that I've been using so far. I'll start adding to it now to explore other options in the IDB cloud. (Non OpenDB requests, object stores, indexes, cursors...)

Created attachment 270964 [details] Basic test with object store This is NOT the basic test I mentioned - it's the basic test + one created object store.

(In reply to comment #5) > Created attachment 270964 [details] > Basic test with object store > > This is NOT the basic test I mentioned - it's the basic test + one created > object store. And, no surprise, creating the object store re-introduces some ref cycles. The object store refs the transaction, so the transaction doesn't die, and the transaction refs the database, so it doesn't die either.

(In reply to comment #6) > (In reply to comment #5) > > Created attachment 270964 [details] > > Basic test with object store > > > > This is NOT the basic test I mentioned - it's the basic test + one created > > object store. > > And, no surprise, creating the object store re-introduces some ref cycles. > > The object store refs the transaction, so the transaction doesn't die, and > the transaction refs the database, so it doesn't die either. The IDBObjectStore needs to keep a reference to the transaction (IDBObjectStore.transaction), and the transaction has to keep references to all of its object stores for IDBTransaction.objectStore(<name>); BUT - It only needs to do so until it's finished. And I doubt it clears out those references. Checking now...

(In reply to comment #7) > (In reply to comment #6) > > (In reply to comment #5) > > > Created attachment 270964 [details] > > > Basic test with object store > > > > > > This is NOT the basic test I mentioned - it's the basic test + one created > > > object store. > > > > And, no surprise, creating the object store re-introduces some ref cycles. > > > > The object store refs the transaction, so the transaction doesn't die, and > > the transaction refs the database, so it doesn't die either. > > The IDBObjectStore needs to keep a reference to the transaction > (IDBObjectStore.transaction), and the transaction has to keep references to > all of its object stores for IDBTransaction.objectStore(<name>); > > BUT - It only needs to do so until it's finished. > > And I doubt it clears out those references. > > Checking now... This was easy to resolve for object stores (https://bugs.webkit.org/show_bug.cgi?id=154061) But the link between object stores and their indexes will be much more difficult to break. As long as the object store is reachable by script, it has to keep all of its referenced indexes alive. Similarly, as long as an index is reachable by script, it has to keep its object store alive. That one will be harder to break.

Note for future self: If we come across some cycle that's "impossible" to break, like possibly the one above, we can at the very least leverage ActiveDOMObject and break them all forcefully when ::stop() is called.

(In reply to comment #9) > Note for future self: If we come across some cycle that's "impossible" to > break, like possibly the one above, we can at the very least leverage > ActiveDOMObject and break them all forcefully when ::stop() is called. This one is definitely possible to break with a great strategy presented by Geoff. But implementing it perfectly is proving problematic. Covered by https://bugs.webkit.org/show_bug.cgi?id=154110

While I hope to get back to exploring more leaks soon, and Darin has expressed interest in exploring them as well, there's still a big elephant in the room: Testing. The strategy implement by Google back when V8 was in the project was "observeGC" It went as follows: objectObserver = internals.observeGC(object); object = null; gc(); shouldBeTrue("objectObserver"); If we had exactly this, that would be fine. If we had any other mechanism that allowed for notification or inspection as to whether or not a particular object was GC'ed, that would be fine as well. Whichever we go with, it seems extremely likely we'd need direct JSC support.

https://bugs.webkit.org/show_bug.cgi?id=158004 likely introduced some more leaks from Workers

I've been given a hint on how to implement observeGC-like functionality using existing JSC API. Filed https://bugs.webkit.org/show_bug.cgi?id=158093 for that

<rdar://problem/27445937>