Changeset 247784 in webkit
- Timestamp:
- Jul 24, 2019 12:21:49 PM (5 years ago)
- Location:
- trunk/Source/WebKit
- Files:
-
- 5 edited
Legend:
- Unmodified
- Added
- Removed
-
trunk/Source/WebKit/ChangeLog
r247782 r247784 1 2019-07-24 Chris Dumez <cdumez@apple.com> 2 3 Crash under WebKit:WTF::Detail::CallableWrapper<WebKit::ResourceLoadStatisticsMemoryStore::updateCookieBlocking(WTF::CompletionHandler<void ()>&&)::$_32::operator()()::'lambda'(), void>::call 4 https://bugs.webkit.org/show_bug.cgi?id=200071 5 <rdar://problem/53335583> 6 7 Reviewed by Brent Fulgham and Youenn Fablet. 8 9 The WebResourceLoadStatisticsStore is a main thread object. In its destructor, it was dispatching 10 to the background queue to destroy the m_statisticsStore / m_persistentStorage data members, which 11 live on the background queue. It would then synchronously wait for the background queue to finish 12 destroying them. The idea was to guarantee that the ResourceLoadStatisticsMemoryStore and the 13 ResourceLoadStatisticsPersistentStorage would never outlive the WebResourceLoadStatisticsStore, 14 given that they keep a raw pointer back to the WebResourceLoadStatisticsStore (via m_store data 15 member). 16 17 The issue is that *while* the WebResourceLoadStatisticsStore destructor is running on the main 18 thread, the background queue may be running code in ResourceLoadStatisticsMemoryStore or 19 ResourceLoadStatisticsPersistentStorage which refs the WebResourceLoadStatisticsStore, even 20 though its ref count has already reached 0. It is actually a common pattern in 21 ResourceLoadStatisticsMemoryStore to call RunLoop::main().dispatch() and ref their m_store in 22 the lambda, so that they can interact with the WebResourceLoadStatisticsStore. 23 24 To address the issue, we now destroy m_statisticsStore / m_persistentStorage *before* the 25 WebResourceLoadStatisticsStore destructor runs. The NetworkSession destructor now calls 26 WebResourceLoadStatisticsStore::didDestroyNetworkSession() which takes care of destroying 27 m_statisticsStore / m_persistentStorage on the background queue, synchronously. The 28 WebResourceLoadStatisticsStore destructor will only run later, once all remaining references 29 to it are gone. 30 31 * NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp: 32 (WebKit::WebResourceLoadStatisticsStore::~WebResourceLoadStatisticsStore): 33 (WebKit::WebResourceLoadStatisticsStore::didDestroyNetworkSession): 34 * NetworkProcess/Classifier/WebResourceLoadStatisticsStore.h: 35 * NetworkProcess/NetworkSession.cpp: 36 (WebKit::NetworkSession::~NetworkSession): 37 1 38 2019-07-24 Youenn Fablet <youenn@apple.com> 2 39 -
trunk/Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp
r247769 r247784 181 181 { 182 182 ASSERT(RunLoop::isMain()); 183 183 ASSERT(!m_statisticsStore); 184 ASSERT(!m_persistentStorage); 185 } 186 187 void WebResourceLoadStatisticsStore::didDestroyNetworkSession() 188 { 189 ASSERT(RunLoop::isMain()); 190 191 m_networkSession = nullptr; 184 192 flushAndDestroyPersistentStore(); 185 193 } … … 336 344 case StorageAccessStatus::RequiresUserPrompt: 337 345 { 346 if (!m_networkSession) 347 return completionHandler(StorageAccessWasGranted::No, StorageAccessPromptWasShown::No); 348 338 349 CompletionHandler<void(bool)> requestConfirmationCompletionHandler = [this, protectedThis = protectedThis.copyRef(), subFrameDomain, topFrameDomain, frameID, pageID, completionHandler = WTFMove(completionHandler)] (bool userDidGrantAccess) mutable { 339 350 if (userDidGrantAccess) … … 359 370 } 360 371 361 if (m_statisticsStore) { 362 m_statisticsStore->requestStorageAccess(WTFMove(subFrameDomain), WTFMove(topFrameDomain), frameID, pageID, [statusHandler = WTFMove(statusHandler)](StorageAccessStatus status) mutable { 363 postTaskReply([statusHandler = WTFMove(statusHandler), status]() mutable { 364 statusHandler(status); 365 }); 372 m_statisticsStore->requestStorageAccess(WTFMove(subFrameDomain), WTFMove(topFrameDomain), frameID, pageID, [statusHandler = WTFMove(statusHandler)](StorageAccessStatus status) mutable { 373 postTaskReply([statusHandler = WTFMove(statusHandler), status]() mutable { 374 statusHandler(status); 366 375 }); 367 } 376 }); 368 377 }); 369 378 } -
trunk/Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.h
r247769 r247784 94 94 95 95 ~WebResourceLoadStatisticsStore(); 96 97 void didDestroyNetworkSession(); 96 98 97 99 static const OptionSet<WebsiteDataType>& monitoredDataTypes(); -
trunk/Source/WebKit/NetworkProcess/NetworkSession.cpp
r247769 r247784 108 108 NetworkSession::~NetworkSession() 109 109 { 110 #if ENABLE(RESOURCE_LOAD_STATISTICS) 111 destroyResourceLoadStatistics(); 112 #endif 113 110 114 m_storageManager->resume(); 111 115 m_storageManager->waitUntilTasksFinished(); 112 116 } 113 117 118 #if ENABLE(RESOURCE_LOAD_STATISTICS) 119 void NetworkSession::destroyResourceLoadStatistics() 120 { 121 if (!m_resourceLoadStatistics) 122 return; 123 124 m_resourceLoadStatistics->didDestroyNetworkSession(); 125 m_resourceLoadStatistics = nullptr; 126 } 127 #endif 128 114 129 void NetworkSession::invalidateAndCancel() 115 130 { … … 130 145 ASSERT(!m_isInvalidated); 131 146 if (!enable) { 132 m_resourceLoadStatistics = nullptr;147 destroyResourceLoadStatistics(); 133 148 return; 134 149 } -
trunk/Source/WebKit/NetworkProcess/NetworkSession.h
r247567 r247784 117 117 NetworkSession(NetworkProcess&, const NetworkSessionCreationParameters&); 118 118 119 #if ENABLE(RESOURCE_LOAD_STATISTICS) 120 void destroyResourceLoadStatistics(); 121 #endif 122 119 123 PAL::SessionID m_sessionID; 120 124 Ref<NetworkProcess> m_networkProcess;
Note: See TracChangeset
for help on using the changeset viewer.