Title: [247112] trunk/Source/WebKit
- Revision
- 247112
- Author
- cdu...@apple.com
- Date
- 2019-07-03 15:16:06 -0700 (Wed, 03 Jul 2019)
Log Message
Fix a couple of thread safety issues in ResourceLoadStatisticsStore
https://bugs.webkit.org/show_bug.cgi?id=199463
Reviewed by Alex Christensen.
The ResourceLoadStatisticsStore object is constructed / used / destroyed on a background queue.
It is therefore not safe to use a WeakPtr to the ResourceLoadStatisticsStore on the main thread.
The safe pattern is to have the ResourceLoadStatisticsStore capture a Ref<> of its m_store before
dispatching to the main thread and use this store on the main thread instead of weakThis->m_store.
ResourceLoadStatisticsStore's m_store is constructed / used / destroyed on the main thread.
* NetworkProcess/Classifier/ResourceLoadStatisticsStore.cpp:
(WebKit::ResourceLoadStatisticsStore::removeDataRecords):
(WebKit::ResourceLoadStatisticsStore::processStatisticsAndDataRecords):
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (247111 => 247112)
--- trunk/Source/WebKit/ChangeLog 2019-07-03 22:14:04 UTC (rev 247111)
+++ trunk/Source/WebKit/ChangeLog 2019-07-03 22:16:06 UTC (rev 247112)
@@ -1,3 +1,21 @@
+2019-07-03 Chris Dumez <cdu...@apple.com>
+
+ Fix a couple of thread safety issues in ResourceLoadStatisticsStore
+ https://bugs.webkit.org/show_bug.cgi?id=199463
+
+ Reviewed by Alex Christensen.
+
+ The ResourceLoadStatisticsStore object is constructed / used / destroyed on a background queue.
+ It is therefore not safe to use a WeakPtr to the ResourceLoadStatisticsStore on the main thread.
+
+ The safe pattern is to have the ResourceLoadStatisticsStore capture a Ref<> of its m_store before
+ dispatching to the main thread and use this store on the main thread instead of weakThis->m_store.
+ ResourceLoadStatisticsStore's m_store is constructed / used / destroyed on the main thread.
+
+ * NetworkProcess/Classifier/ResourceLoadStatisticsStore.cpp:
+ (WebKit::ResourceLoadStatisticsStore::removeDataRecords):
+ (WebKit::ResourceLoadStatisticsStore::processStatisticsAndDataRecords):
+
2019-07-03 Youenn Fablet <you...@apple.com>
Isolate CacheStorage::Engine path when hopping to a background thread
Modified: trunk/Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsStore.cpp (247111 => 247112)
--- trunk/Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsStore.cpp 2019-07-03 22:14:04 UTC (rev 247111)
+++ trunk/Source/WebKit/NetworkProcess/Classifier/ResourceLoadStatisticsStore.cpp 2019-07-03 22:16:06 UTC (rev 247112)
@@ -205,13 +205,8 @@
setDataRecordsBeingRemoved(true);
- RunLoop::main().dispatch([domainsToRemoveWebsiteDataFor = crossThreadCopy(domainsToRemoveWebsiteDataFor), completionHandler = WTFMove(completionHandler), weakThis = makeWeakPtr(*this), shouldNotifyPagesWhenDataRecordsWereScanned = m_parameters.shouldNotifyPagesWhenDataRecordsWereScanned, workQueue = m_workQueue.copyRef()] () mutable {
- if (!weakThis) {
- completionHandler();
- return;
- }
-
- weakThis->m_store.deleteWebsiteDataForRegistrableDomains(WebResourceLoadStatisticsStore::monitoredDataTypes(), WTFMove(domainsToRemoveWebsiteDataFor), shouldNotifyPagesWhenDataRecordsWereScanned, [completionHandler = WTFMove(completionHandler), weakThis = WTFMove(weakThis), workQueue = workQueue.copyRef()](const HashSet<RegistrableDomain>& domainsWithDeletedWebsiteData) mutable {
+ RunLoop::main().dispatch([store = makeRef(m_store), domainsToRemoveWebsiteDataFor = crossThreadCopy(domainsToRemoveWebsiteDataFor), completionHandler = WTFMove(completionHandler), weakThis = makeWeakPtr(*this), shouldNotifyPagesWhenDataRecordsWereScanned = m_parameters.shouldNotifyPagesWhenDataRecordsWereScanned, workQueue = m_workQueue.copyRef()] () mutable {
+ store->deleteWebsiteDataForRegistrableDomains(WebResourceLoadStatisticsStore::monitoredDataTypes(), WTFMove(domainsToRemoveWebsiteDataFor), shouldNotifyPagesWhenDataRecordsWereScanned, [completionHandler = WTFMove(completionHandler), weakThis = WTFMove(weakThis), workQueue = workQueue.copyRef()](const HashSet<RegistrableDomain>& domainsWithDeletedWebsiteData) mutable {
workQueue->dispatch([domainsWithDeletedWebsiteData = crossThreadCopy(domainsWithDeletedWebsiteData), completionHandler = WTFMove(completionHandler), weakThis = WTFMove(weakThis)] () mutable {
if (!weakThis) {
completionHandler();
@@ -247,12 +242,8 @@
if (!m_parameters.shouldNotifyPagesWhenDataRecordsWereScanned)
return;
- RunLoop::main().dispatch([this, weakThis = WTFMove(weakThis)] {
- ASSERT(RunLoop::isMain());
- if (!weakThis)
- return;
-
- m_store.notifyResourceLoadStatisticsProcessed();
+ RunLoop::main().dispatch([store = makeRef(m_store)] {
+ store->notifyResourceLoadStatisticsProcessed();
});
});
}
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
