WebResourceLoadStatisticsStore::m_operatingDates is unsafely modified from several...
authorcdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 21 Jul 2017 20:40:13 +0000 (20:40 +0000)
committercdumez@apple.com <cdumez@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 21 Jul 2017 20:40:13 +0000 (20:40 +0000)
https://bugs.webkit.org/show_bug.cgi?id=174721
<rdar://problem/33400343>

Reviewed by Brent Fulgham.

WebResourceLoadStatisticsStore::m_operatingDates is supposed to only be modified on
the background thread. However, WebResourceLoadStatisticsStore::performDailyTasks()
was mistakenly calling includeTodayAsOperatingDateIfNecessary() on the main thread,
which would modify m_operatingDates. This could lead to crashes such as the
one in <rdar://problem/33400343>, as the main thread may modify m_operatingDates
while we are interating over it on the background thread to save it to disk.

* UIProcess/WebResourceLoadStatisticsStore.cpp:
(WebKit::WebResourceLoadStatisticsStore::performDailyTasks):
(WebKit::WebResourceLoadStatisticsStore::includeTodayAsOperatingDateIfNecessary):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@219739 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebKit/ChangeLog
Source/WebKit/UIProcess/WebResourceLoadStatisticsStore.cpp

index 71edfa1..0c65577 100644 (file)
@@ -1,3 +1,22 @@
+2017-07-21  Chris Dumez  <cdumez@apple.com>
+
+        WebResourceLoadStatisticsStore::m_operatingDates is unsafely modified from several threads
+        https://bugs.webkit.org/show_bug.cgi?id=174721
+        <rdar://problem/33400343>
+
+        Reviewed by Brent Fulgham.
+
+        WebResourceLoadStatisticsStore::m_operatingDates is supposed to only be modified on
+        the background thread. However, WebResourceLoadStatisticsStore::performDailyTasks()
+        was mistakenly calling includeTodayAsOperatingDateIfNecessary() on the main thread,
+        which would modify m_operatingDates. This could lead to crashes such as the
+        one in <rdar://problem/33400343>, as the main thread may modify m_operatingDates
+        while we are interating over it on the background thread to save it to disk.
+
+        * UIProcess/WebResourceLoadStatisticsStore.cpp:
+        (WebKit::WebResourceLoadStatisticsStore::performDailyTasks):
+        (WebKit::WebResourceLoadStatisticsStore::includeTodayAsOperatingDateIfNecessary):
+
 2017-07-21  Brady Eidson  <beidson@apple.com>
 
         Get rid of WebCore IconDatabase code.
 2017-07-21  Brady Eidson  <beidson@apple.com>
 
         Get rid of WebCore IconDatabase code.
index 4e2881a..7e61913 100644 (file)
@@ -281,7 +281,9 @@ void WebResourceLoadStatisticsStore::performDailyTasks()
 {
     ASSERT(RunLoop::isMain());
 
 {
     ASSERT(RunLoop::isMain());
 
-    includeTodayAsOperatingDateIfNecessary();
+    m_statisticsQueue->dispatch([this, protectedThis = makeRef(*this)] {
+        includeTodayAsOperatingDateIfNecessary();
+    });
     if (m_parameters.shouldSubmitTelemetry)
         submitTelemetry();
 }
     if (m_parameters.shouldSubmitTelemetry)
         submitTelemetry();
 }
@@ -732,6 +734,8 @@ Vector<String> WebResourceLoadStatisticsStore::topPrivatelyControlledDomainsToRe
 
 void WebResourceLoadStatisticsStore::includeTodayAsOperatingDateIfNecessary()
 {
 
 void WebResourceLoadStatisticsStore::includeTodayAsOperatingDateIfNecessary()
 {
+    ASSERT(!RunLoop::isMain());
+
     auto today = OperatingDate::today();
     if (!m_operatingDates.isEmpty() && today <= m_operatingDates.last())
         return;
     auto today = OperatingDate::today();
     if (!m_operatingDates.isEmpty() && today <= m_operatingDates.last())
         return;