Bug 102298 – IndexedDB: database connections don't close after versionchange transaction aborts

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[David Grogan]

Paraphrased from "4.1 Opening a database", emphasis added.

7. If the version of db is lower than version, run the steps for running a "versionchange" transaction

8. If the previous step resulted in an error ... ENSURE THAT CONNECTION IS CLOSED by running the steps for closing a database connection.

Chrome doesn't ensure the connections are closed.  See the lazy-index-population layout test.

[David Grogan]

Chromium bug: http://code.google.com/p/chromium/issues/detail?id=161066

[Joshua Bell]

Created attachment 187373 [details]
Patch

[Joshua Bell]

The patch looks simple, but I actually tried several other approaches first, such as initiating the close from the IDBOpenDBRequest::onError() handler, or the IDBDatabaseBackendImpl::transactionFinishedAndAbortFired(). Neither of those quite work.

I should probably add a dedicated test, although this is covered indirectly.

[David Grogan]

Comment on attachment 187373 [details]
Patch

lgtm

The other approaches don't quite work because they are too disruptive to the assumptions our code makes about the state of data structures during the open/close/upgradeneeded/complete/abort dance?

[Joshua Bell]

(In reply to comment #4)
> The other approaches don't quite work because they are too disruptive to the assumptions our code makes about the state of data structures during the open/close/upgradeneeded/complete/abort dance?

Yep. Initiating from the back-end leaves the front-end out of sync so it may call backend->close() again which isn't idempotent. (This could be changed to go through the force-close route, I suppose.) I don't recall why starting from IDBOpenDBRequest::onError didn't pan out - it was the first thing I tried and I actually had this patch laying around for a while and just dusted it off today.

The important thing is that you can't do anything with the database before the close happens. I'll verify this with a test, but from inspection: if upgradeneeded fires the IDBDatabase has a version change transaction, so will not let you initiate another transaction. That doesn't get cleared out until that transaction's onComplete or onAbort fires. It's during the onAbort that the close is initiated, and the close should "complete" once the transaction tells the database it's finished a few steps later.

[Joshua Bell]

Created attachment 187608 [details]
Patch

[Joshua Bell]

tony@ - can your review?

[WebKit Review Bot]

Comment on attachment 187608 [details]
Patch

Clearing flags on attachment: 187608

Committed r142513: <http://trac.webkit.org/changeset/142513>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.