WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
89512
[Chromium] IndexedDB: Don't close database if pending connections are in flight
https://bugs.webkit.org/show_bug.cgi?id=89512
Summary
[Chromium] IndexedDB: Don't close database if pending connections are in flight
Joshua Bell
Reported
2012-06-19 13:59:49 PDT
If the IDB front IDBDatabaseBackendImpl, IDBRequest and IDBDatabase objects are communicating asynchronously (e.g. Chromium port), then there is a race condition: A normal connection opens - note the two phase openConnection() / registerFrontendCallbacks() * IDBRequest req1 = IDBFactory::open() * which async invokes IDBDatabaseBackendImpl::openConnection() * which async invokes IDBRequest::OnSuccess() on req1, which creates IDBDatabase connection1 * which async invokes IDBDatabaseBackendImpl::registerFrontendCallbacks(IDBDatabase), which "registers" connection1 Now a second connection opens, where * IDBRequest req2 = IDBFactory::open() * which async invokes IDBDatabaseBackendImpl::openConnection() * which async invokes IDBRequest::OnSuccess() on req2, which creates IDBDatabase connection2 * which async invokes IDBDatabaseBackendImpl::registerFrontendCallbacks(IDBDatabase)... * INTERRUPT: * connection1 async invokes IDBDatabase::close() which calls IDBDatabaseBackendImpl::close() * at this point there are no registered callbacks, so it releases its backing store and drops itself from the factory * RESUME: * IDBDatabaseBackendImpl::registerFrontendCallbacks(IDBDatabase) "registers" connection2 * connection2.setVersion() is called, but the backing store ref has been released. *boom* So the bug is that IDBDatabaseBackendImpl::close should not close itself if there are pending connections in the limbo state between openConnection() and registerFrontendCallbacks(). The correct long term fix is probably to eliminate this limbo state entirely.
Attachments
Patch
(19.09 KB, patch)
2012-06-19 14:53 PDT
,
Joshua Bell
no flags
Details
Formatted Diff
Diff
Patch for landing
(18.72 KB, patch)
2012-06-19 17:54 PDT
,
Joshua Bell
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Joshua Bell
Comment 1
2012-06-19 14:53:12 PDT
Created
attachment 148432
[details]
Patch
Joshua Bell
Comment 2
2012-06-19 14:54:45 PDT
+vsevik@ for the inspector changes. This patch requires that the second phase of database connections is processed.
Joshua Bell
Comment 3
2012-06-19 14:55:16 PDT
tony@ can you take a look?
Tony Chang
Comment 4
2012-06-19 15:23:13 PDT
Comment on
attachment 148432
[details]
Patch LGTM
Joshua Bell
Comment 5
2012-06-19 15:32:17 PDT
dgrogan@, can you take a look too since we were discussing this and how it would relate to your upcoming patch?
David Grogan
Comment 6
2012-06-19 17:03:24 PDT
Comment on
attachment 148432
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=148432&action=review
LGTM
> Source/WebCore/Modules/indexeddb/IDBDatabaseBackendImpl.cpp:347 > + RefPtr<IDBDatabaseBackendImpl> self = this;
What scenario does this prevent?
David Grogan
Comment 7
2012-06-19 17:05:16 PDT
I didn't look at the inspector part at all.
Joshua Bell
Comment 8
2012-06-19 17:53:32 PDT
Comment on
attachment 148432
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=148432&action=review
>> Source/WebCore/Modules/indexeddb/IDBDatabaseBackendImpl.cpp:347 >> + RefPtr<IDBDatabaseBackendImpl> self = this; > > What scenario does this prevent?
Whoops, good catch. That's just residue from early debugging.
Joshua Bell
Comment 9
2012-06-19 17:54:09 PDT
Created
attachment 148474
[details]
Patch for landing
WebKit Review Bot
Comment 10
2012-06-20 09:18:09 PDT
Comment on
attachment 148474
[details]
Patch for landing Clearing flags on attachment: 148474 Committed
r120828
: <
http://trac.webkit.org/changeset/120828
>
WebKit Review Bot
Comment 11
2012-06-20 09:18:13 PDT
All reviewed patches have been landed. Closing bug.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug