Bug 102298

Summary:

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

Product:

WebKit

Reporter:

David Grogan <dgrogan>

Component:

WebCore Misc.

Assignee:

Joshua Bell <jsbell>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

alecflett, jsbell, tony, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none

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

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

Comment 2 Joshua Bell 2013-02-08 15:46:57 PST

Created attachment 187373 [details]
Patch

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 6 Joshua Bell 2013-02-11 10:13:11 PST

Created attachment 187608 [details]
Patch

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.