Changeset 248393 in webkit

Timestamp:

Aug 7, 2019 3:29:18 PM (5 years ago)

Author:

Chris Dumez

Message:

Regression(r247784) ResourceLoadStatisticsMemoryStore / ResourceLoadStatisticsPersistentStorage may get destroyed on the wrong thread
​https://bugs.webkit.org/show_bug.cgi?id=200517

Reviewed by Geoffrey Garen.

The issue is that WebResourceLoadStatisticsStore::flushAndDestroyPersistentStore() is null checking
m_persistentStorage and m_statisticsStore on the main thread, even though those members are initialized
and destroyed on the background thread. As a result, if flushAndDestroyPersistentStore() is called *before*
the background task to initialize those members has had a chance to run, then we'd return early without
destroying those members. Later on, the background task would then initialize those data members and we
would then destroy them on the main thread when the WebResourceLoadStatisticsStore is destroyed on the
main thread.

• NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp:

(WebKit::WebResourceLoadStatisticsStore::flushAndDestroyPersistentStore):

Location:

trunk/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp (modified) (1 diff)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2019-08-07  Chris Dumez  <cdumez@apple.com>
+
+        Regression(r247784) ResourceLoadStatisticsMemoryStore / ResourceLoadStatisticsPersistentStorage may get destroyed on the wrong thread
+        https://bugs.webkit.org/show_bug.cgi?id=200517
+
+        Reviewed by Geoffrey Garen.
+
+        The issue is that WebResourceLoadStatisticsStore::flushAndDestroyPersistentStore() is null checking
+        m_persistentStorage and m_statisticsStore on the main thread, even though those members are initialized
+        and destroyed on the background thread. As a result, if flushAndDestroyPersistentStore() is called *before*
+        the background task to initialize those members has had a chance to run, then we'd return early without
+        destroying those members. Later on, the background task would then initialize those data members and we
+        would then destroy them on the main thread when the WebResourceLoadStatisticsStore is destroyed on the
+        main thread.
+
+        * NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp:
+        (WebKit::WebResourceLoadStatisticsStore::flushAndDestroyPersistentStore):
+
 2019-08-06  Dean Jackson  <dino@apple.com>
 

trunk/Source/WebKit/NetworkProcess/Classifier/WebResourceLoadStatisticsStore.cpp

@@ -211 +211 @@
     ASSERT(RunLoop::isMain());
 
-    if (!m_persistentStorage && !m_statisticsStore)
-        return;
-
     // Make sure we destroy the persistent store on the background queue and wait for it to die
     // synchronously since it has a C++ reference to us. Blocking nature of this task allows us