Bug 123313 - DatabaseManager's ProposedDatabases need to be thread-safe
Summary: DatabaseManager's ProposedDatabases need to be thread-safe
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: DOM (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Lam
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2013-10-24 19:29 PDT by Simon Fraser (smfr)
Modified: 2013-10-25 15:04 PDT (History)
5 users (show)

See Also:


Attachments
the patch. (9.39 KB, patch)
2013-10-25 12:45 PDT, Mark Lam
ggaren: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Simon Fraser (smfr) 2013-10-24 19:29:40 PDT
We're seeing some recent crashes in database-related code, e.g.

http://build.webkit.org/results/Apple%20MountainLion%20Debug%20WK2%20(Tests)/r157967%20(13656)/http/tests/security/cross-origin-worker-websql-allowed-crash-log.txt

Thread 30 Crashed:: WebCore: Worker
0   com.apple.JavaScriptCore      	0x000000010d280eba WTFCrash + 42 (Assertions.cpp:342)
1   com.apple.WebCore             	0x000000010e5b58e8 WebCore::DatabaseManager::ProposedDatabase::ProposedDatabase(WebCore::DatabaseManager&, WebCore::SecurityOrigin*, WTF::String const&, WTF::String const&, unsigned long) + 248 (DatabaseManager.cpp:58)
2   com.apple.WebCore             	0x000000010e5b57dd WebCore::DatabaseManager::ProposedDatabase::ProposedDatabase(WebCore::DatabaseManager&, WebCore::SecurityOrigin*, WTF::String const&, WTF::String const&, unsigned long) + 61 (DatabaseManager.cpp:60)
3   com.apple.WebCore             	0x000000010e5b641d WebCore::DatabaseManager::openDatabaseBackend(WebCore::ScriptExecutionContext*, WebCore::DatabaseType, WTF::String const&, WTF::String const&, WTF::String const&, unsigned long, bool, WebCore::DatabaseError&, WTF::String&) + 765 (DatabaseManager.cpp:268)
4   com.apple.WebCore             	0x000000010e5b6929 WebCore::DatabaseManager::openDatabase(WebCore::ScriptExecutionContext*, WTF::String const&, WTF::String const&, WTF::String const&, unsigned long, WTF::PassRefPtr<WebCore::DatabaseCallback>, WebCore::DatabaseError&) + 297 (DatabaseManager.cpp:309)
5   com.apple.WebCore             	0x000000010fc48d78 WebCore::WorkerGlobalScopeWebDatabase::openDatabase(WebCore::WorkerGlobalScope*, WTF::String const&, WTF::String const&, WTF::String const&, unsigned long, WTF::PassRefPtr<WebCore::DatabaseCallback>, int&) + 328 (WorkerGlobalScopeWebDatabase.cpp:49)
6   com.apple.WebCore             	0x000000010f29758d WebCore::jsWorkerGlobalScopePrototypeFunctionOpenDatabase(JSC::ExecState*) + 1629 (JSWorkerGlobalScope.cpp:1167)
7   ???                           	0x00005d6b24a15e25 0 + 102714757439013
8   com.apple.JavaScriptCore      	0x000000010cfb6267 JSC::JITCode::execute(JSC::JSStack*, JSC::ExecState*, JSC::VM*) + 71 (JITCode.cpp:46)
9   com.apple.JavaScriptCore      	0x000000010cf9a01f JSC::Interpreter::executeCall(JSC::ExecState*, JSC::JSObject*, JSC::CallType, JSC::CallData const&, JSC::JSValue, JSC::ArgList const&) + 1455 (Interpreter.cpp:958)
Comment 2 Mark Lam 2013-10-25 09:50:26 PDT
Looks like the ProposedDatabases need to be multi-thread safe as well due to Worker threads.  Will fix.
Comment 3 Radar WebKit Bug Importer 2013-10-25 10:36:49 PDT
<rdar://problem/15320813>
Comment 4 Mark Lam 2013-10-25 12:45:13 PDT
Created attachment 215199 [details]
the patch.

Still running layout tests on this patch.
Comment 5 Mark Lam 2013-10-25 13:42:28 PDT
Layout tests look ok.
Comment 6 Geoffrey Garen 2013-10-25 14:22:23 PDT
Comment on attachment 215199 [details]
the patch.

View in context: https://bugs.webkit.org/attachment.cgi?id=215199&action=review

> Source/WebCore/Modules/webdatabase/DatabaseManager.cpp:257
> +                addProposedDatabase(&proposedDb);
>                  databaseContext->databaseExceededQuota(name,
>                      DatabaseDetails(name.isolatedCopy(), displayName.isolatedCopy(), estimatedSize, 0));
> +                removeProposedDatabase(&proposedDb);

Let's move add/remove to the ProposedDatabase constructor/destructor, to make them automatic.
Comment 7 Mark Lam 2013-10-25 15:04:49 PDT
Thanks for the review.  The add and remove calls have been moved into ProposedDatabase's constructor/destructor.

Landed in r158058: <http://trac.webkit.org/r158058>.