Summary: | Crashed while ref'ing DatabaseContext in DatabaseManager::interruptAllDatabasesForContext() | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Keishi Hattori <keishi> | ||||
Component: | Tools / Tests | Assignee: | Mark Lam <mark.lam> | ||||
Status: | RESOLVED FIXED | ||||||
Severity: | Normal | CC: | alancutter, ap, beidson, ggaren, levin, levin+threading, mark.lam, sam, webkit.review.bot | ||||
Priority: | P2 | ||||||
Version: | 528+ (Nightly build) | ||||||
Hardware: | Unspecified | ||||||
OS: | Unspecified | ||||||
Attachments: |
|
Description
Keishi Hattori
2013-01-29 19:35:11 PST
WebCore::DatabaseManager::interruptAllDatabasesForContext() is the only API that can access another thread's DatabaseContext. Since DatabaseContext is ref counted and can be ref'ed by another thread (in the interrupt case), it should extend ThreadSafeRefCounted instead of the RefCounted. Investigating the fix right now. Created attachment 185565 [details]
The fix.
Comment on attachment 185565 [details] The fix. View in context: https://bugs.webkit.org/attachment.cgi?id=185565&action=review > Source/WebCore/ChangeLog:14 > + This reflects the contract that another thread (calling doing the Typo in comment. Will remove "calling". Comment on attachment 185565 [details] The fix. View in context: https://bugs.webkit.org/attachment.cgi?id=185565&action=review r=me on ThreadSafeRefCounted part. > Source/WebCore/Modules/webdatabase/DatabaseManager.cpp:353 > -void DatabaseManager::interruptAllDatabasesForContext(ScriptExecutionContext* context) > +void DatabaseManager::interruptAllDatabasesForContext(const ScriptExecutionContext* context) I do not think that we should be using "const ScriptExecutionContext*" here, or anywhere. These are huge "world" objects that are never actually immutable, and saying that they are constant for the purposes of a particular function does not have any semantic meaning that I could catch. For example, you are passing context as constant here. But interrupting all databases for context modifies the context in a very noticeable way! Removed "const" changes. Landed in r141320: <http://trac.webkit.org/changeset/141320>. Thanks Mark, the Chromium debug tests are happily passing again. Chromium expectations updated on: http://trac.webkit.org/changeset/141365 |