Crash under WebKit: WTF::Function<void ()>::CallableWrapper<WebKit::ResourceLoadStatisticsMemoryStore::removeDataRecords(WTF::CompletionHandler<void ()>&&)::$_1>::call(): Thread 0 name: Dispatch queue: com.apple.main-thread Thread 0 Crashed: 0 WebKit 0x00000002208e65c8 WTF::Function<void ()>::CallableWrapper<WebKit::ResourceLoadStatisticsMemoryStore::removeDataRecords(WTF::CompletionHandler<void ()>&&)::$_1>::call() + 76 1 WebKit 0x00000002208e65a8 WTF::Function<void ()>::CallableWrapper<WebKit::ResourceLoadStatisticsMemoryStore::removeDataRecords(WTF::CompletionHandler<void ()>&&)::$_1>::call() + 44 2 JavaScriptCore 0x00000002181780f0 WTF::RunLoop::performWork() + 276 3 JavaScriptCore 0x00000002181783b8 WTF::RunLoop::performWork(void*) + 36 4 CoreFoundation 0x0000000210e365b8 __CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 24 5 CoreFoundation 0x0000000210e36538 __CFRunLoopDoSource0 + 88 6 CoreFoundation 0x0000000210e35e1c __CFRunLoopDoSources0 + 176 7 CoreFoundation 0x0000000210e30ce8 __CFRunLoopRun + 1040 8 CoreFoundation 0x0000000210e305b8 CFRunLoopRunSpecific + 436 9 GraphicsServices 0x00000002130a3584 GSEventRunModal + 100 10 UIKitCore 0x000000023df892dc UIApplicationMain + 212 11 SafariViewService 0x000000010006a8bc main + 244 12 libdyld.dylib 0x00000002108f0b94 start + 4
<rdar://problem/43179891>
Created attachment 348434 [details] Patch
Comment on attachment 348434 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=348434&action=review > Source/WebKit/UIProcess/ResourceLoadStatisticsMemoryStore.cpp:246 > It seems that callback here is a completion handler. There is no guarantee though that it will be called if the workQueue is stopped. Maybe it should be WTF::Function(). Also, is it fine that callback be destroyed in the main thread? From my reading of the code, it seems ok right now since the passed callbacks are capturing weakThis, but it would be quite easy to break that assumption. Maybe removeDataRecords should not take a callback at all. Instead, it could create the callback itself when being called, this would remove this double weakThis check and be more future proof.
(In reply to youenn fablet from comment #3) > Comment on attachment 348434 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=348434&action=review > > > Source/WebKit/UIProcess/ResourceLoadStatisticsMemoryStore.cpp:246 > > > > It seems that callback here is a completion handler. > There is no guarantee though that it will be called if the workQueue is > stopped. Maybe it should be WTF::Function(). > > Also, is it fine that callback be destroyed in the main thread? > From my reading of the code, it seems ok right now since the passed > callbacks are capturing weakThis, but it would be quite easy to break that > assumption. > > Maybe removeDataRecords should not take a callback at all. > Instead, it could create the callback itself when being called, this would > remove this double weakThis check and be more future proof. If you do a WorkQueue::dispatch(c), c is *guaranteed* to run.
Comment on attachment 348434 [details] Patch Clearing flags on attachment: 348434 Committed r235487: <https://trac.webkit.org/changeset/235487>
All reviewed patches have been landed. Closing bug.