|Summary:||IndexedDB: database connections don't close after versionchange transaction aborts|
|Product:||WebKit||Reporter:||David Grogan <dgrogan>|
|Component:||WebCore Misc.||Assignee:||Joshua Bell <jsbell>|
|Severity:||Normal||CC:||alecflett, jsbell, tony, webkit.review.bot|
|Version:||528+ (Nightly build)|
Description David Grogan 2012-11-14 16:22:09 PST
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.
Comment 1 David Grogan 2012-11-14 16:22:34 PST
Comment 3 Joshua Bell 2013-02-08 15:50:11 PST
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.
Comment 4 David Grogan 2013-02-08 17:02:48 PST
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?
Comment 5 Joshua Bell 2013-02-08 17:32:12 PST
(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.
Comment 7 Joshua Bell 2013-02-11 10:13:35 PST
tony@ - can your review?
Comment 8 WebKit Review Bot 2013-02-11 14:14:58 PST
Comment on attachment 187608 [details] Patch Clearing flags on attachment: 187608 Committed r142513: <http://trac.webkit.org/changeset/142513>
Comment 9 WebKit Review Bot 2013-02-11 14:15:01 PST
All reviewed patches have been landed. Closing bug.