Bug 199599

Summary: Speculative fix for crashes under LocalStorageDatabaseTracker::databasePath()
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: WebKit2Assignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, beidson, rniwa, sihui_liu, webkit-bug-importer, youennf
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch rniwa: review+

Description Chris Dumez 2019-07-08 16:09:14 PDT
Speculative fix for crashes under LocalStorageDatabaseTracker::databasePath():
Thread 2 name:  Dispatch queue: com.apple.WebKit.StorageManager
Thread 2 Crashed ↩:
0   CoreFoundation                	0x00000001a2d33458 __CFStringCreateImmutableFunnel3 + 688 (CFString.c:1617)
1   CoreFoundation                	0x00000001a2d3320c __CFStringCreateImmutableFunnel3 + 100 (CFInternal.h:390)
2   CoreFoundation                	0x00000001a2d33e20 CFStringCreateWithCharacters + 60 (CFString.c:1873)
3   JavaScriptCore                	0x00000001b22f5ef8 WTF::StringImpl::createCFString() + 140 (StringImplCF.cpp:128)
4   JavaScriptCore                	0x00000001b22cadc0 WTF::FileSystemImpl::fileSystemRepresentation(WTF::String const&) + 36 (FileSystemCF.cpp:41)
5   JavaScriptCore                	0x00000001b22ccfe0 WTF::FileSystemImpl::makeAllDirectories(WTF::String const&) + 28 (FileSystemPOSIX.cpp:313)
6   WebKit                        	0x00000001aa4b0924 WebKit::LocalStorageDatabaseTracker::databasePath(WTF::String const&) const + 52 (LocalStorageDatabaseTracker.cpp:153)
7   WebKit                        	0x00000001aa4af9d8 WebKit::LocalStorageDatabaseTracker::databasePath(WebCore::SecurityOriginData const&) const + 124 (LocalStorageDatabaseTracker.cpp:66)
8   WebKit                        	0x00000001aa4af934 WebKit::LocalStorageDatabase::LocalStorageDatabase(WTF::Ref<WTF::WorkQueue, WTF::DumbPtrTraits<WTF::WorkQueue> >&&, WTF::Ref<WebKit::LocalStorageDatabaseTracker, WTF::DumbPtrTraits<WebKit::LocalStorageDatabaseTracker> >&&, WebCore::SecurityOriginData const&) + 144 (LocalStorageDatabase.cpp:57)
9   WebKit                        	0x00000001aa4b1558 WebKit::StorageManager::StorageArea::openDatabaseAndImportItemsIfNeeded() const + 180 (LocalStorageDatabase.cpp:63)
10  WebKit                        	0x00000001aa4bec34 WTF::Detail::CallableWrapper<WebKit::StorageManager::getValues(IPC::Connection&, WebCore::SecurityOriginData&&, unsigned long long, unsigned long long, WTF::CompletionHandler<void (WTF::HashMap<WTF::String, WTF::String, WTF::StringHash, WTF::HashTraits<WTF::String>, WTF::HashTraits<WTF::String> > const&)>&&)::$_27, void>::call() + 52 (StorageManager.cpp:289)
11  JavaScriptCore                	0x00000001b232c3cc WTF::BlockPtr<void ()> WTF::BlockPtr<void ()>::fromCallable<WTF::WorkQueue::dispatch(WTF::Function<void ()>&&)::$_0>(WTF::WorkQueue::dispatch(WTF::Function<void ()>&&)::$_0)::'lambda'(void*)::__invoke(void*) + 32 (Function.h:79)
12  libdispatch.dylib             	0x00000001a2a3ecb0 _dispatch_call_block_and_release + 32 (init.c:1408)
13  libdispatch.dylib             	0x00000001a2a4010c _dispatch_client_callout + 20 (object.m:495)
14  libdispatch.dylib             	0x00000001a2a4652c _dispatch_lane_serial_drain + 568 (inline_internal.h:2487)
15  libdispatch.dylib             	0x00000001a2a46f54 _dispatch_lane_invoke + 364 (inline_internal.h:1854)
16  libdispatch.dylib             	0x00000001a2a4ffc8 _dispatch_workloop_worker_thread + 580 (queue.c:6380)
17  libsystem_pthread.dylib       	0x00000001a2aa5a64 _pthread_wqthread + 308 (pthread.c:2329)
18  libsystem_pthread.dylib       	0x00000001a2aabc44 start_wqthread + 8
Comment 1 Chris Dumez 2019-07-08 16:09:28 PDT
<rdar://problem/31169686>
Comment 2 Chris Dumez 2019-07-08 16:17:01 PDT
Created attachment 373678 [details]
Patch
Comment 3 Ryosuke Niwa 2019-07-08 19:25:37 PDT
Comment on attachment 373678 [details]
Patch

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

> Source/WebKit/NetworkProcess/WebStorage/LocalStorageDatabaseTracker.h:78
> -    String m_localStorageDirectory;
> +    const String m_localStorageDirectory;

Can we add a comment saying why it's not safe to use this in a background thread?
Comment 4 Chris Dumez 2019-07-08 20:06:19 PDT
Committed r247250: <https://trac.webkit.org/changeset/247250>