Fix lifetime management issue in ResourceLoadStatisticsPersistentStorage::scheduleOrW...

https://bugs.webkit.org/show_bug.cgi?id=174790

Reviewed by Brady Eidson.

Fix lifetime management issue in ResourceLoadStatisticsPersistentStorage::scheduleOrWriteMemoryStore().
WorkQueue::dispatchAfter() keeps the WorkQueue alive because its implementation keeps a strong ref to
|this|. As a result, the lambda passed to dispatchAfter(), which calls writeMemoryStoreToDisk(), can
get executed after the store is gone.

* UIProcess/Storage/ResourceLoadStatisticsPersistentStorage.cpp:
(WebKit::ResourceLoadStatisticsPersistentStorage::ResourceLoadStatisticsPersistentStorage):
(WebKit::ResourceLoadStatisticsPersistentStorage::scheduleOrWriteMemoryStore):
* UIProcess/Storage/ResourceLoadStatisticsPersistentStorage.h:
(WebKit::ResourceLoadStatisticsPersistentStorage::createWeakPtr):

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

diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
index fea68e1..c0b87fa 100644 (file)
--- a/Source/WebKit/ChangeLog
+++ b/Source/WebKit/ChangeLog
@@ -1,3 +1,21 @@
+2017-07-24  Chris Dumez  <cdumez@apple.com>
+
+        Fix lifetime management issue in ResourceLoadStatisticsPersistentStorage::scheduleOrWriteMemoryStore()
+        https://bugs.webkit.org/show_bug.cgi?id=174790
+
+        Reviewed by Brady Eidson.
+
+        Fix lifetime management issue in ResourceLoadStatisticsPersistentStorage::scheduleOrWriteMemoryStore().
+        WorkQueue::dispatchAfter() keeps the WorkQueue alive because its implementation keeps a strong ref to
+        |this|. As a result, the lambda passed to dispatchAfter(), which calls writeMemoryStoreToDisk(), can
+        get executed after the store is gone.
+
+        * UIProcess/Storage/ResourceLoadStatisticsPersistentStorage.cpp:
+        (WebKit::ResourceLoadStatisticsPersistentStorage::ResourceLoadStatisticsPersistentStorage):
+        (WebKit::ResourceLoadStatisticsPersistentStorage::scheduleOrWriteMemoryStore):
+        * UIProcess/Storage/ResourceLoadStatisticsPersistentStorage.h:
+        (WebKit::ResourceLoadStatisticsPersistentStorage::createWeakPtr):
+
 2017-07-23  Carlos Garcia Campos  <cgarcia@igalia.com>
 
         Unreviewed. REGRESSION(r219713): [GTK][WPE] Fix default favicon database patch.

diff --git a/Source/WebKit/UIProcess/Storage/ResourceLoadStatisticsPersistentStorage.cpp b/Source/WebKit/UIProcess/Storage/ResourceLoadStatisticsPersistentStorage.cpp
index 58ea1aa..7c80114 100644 (file)
--- a/Source/WebKit/UIProcess/Storage/ResourceLoadStatisticsPersistentStorage.cpp
+++ b/Source/WebKit/UIProcess/Storage/ResourceLoadStatisticsPersistentStorage.cpp
@@ -83,7 +83,8 @@ static std::unique_ptr<KeyedDecoder> createDecoderForFile(const String& path)
 }
 
 ResourceLoadStatisticsPersistentStorage::ResourceLoadStatisticsPersistentStorage(WebResourceLoadStatisticsStore& store, const String& storageDirectoryPath)
-    : m_memoryStore(store)
+    : m_weakPtrFactory(this)
+    , m_memoryStore(store)
     , m_storageDirectoryPath(storageDirectoryPath)
 {
 }
@@ -280,8 +281,10 @@ void ResourceLoadStatisticsPersistentStorage::scheduleOrWriteMemoryStore()
         if (!m_hasPendingWrite) {
             m_hasPendingWrite = true;
             Seconds delay = minimumWriteInterval - timeSinceLastWrite + 1_s;
-            m_memoryStore.statisticsQueue().dispatchAfter(delay, [this] () mutable {
-                writeMemoryStoreToDisk();
+            // WorkQueue::dispatchAfter() keeps the WorkQueue alive so the dispatched lambda may get executed after the store has been destroyed.
+            m_memoryStore.statisticsQueue().dispatchAfter(delay, [weakThis = createWeakPtr()] () mutable {
+                if (weakThis)
+                    weakThis->writeMemoryStoreToDisk();
             });
         }
         return;

diff --git a/Source/WebKit/UIProcess/Storage/ResourceLoadStatisticsPersistentStorage.h b/Source/WebKit/UIProcess/Storage/ResourceLoadStatisticsPersistentStorage.h
index e3544f5..5453be8 100644 (file)
--- a/Source/WebKit/UIProcess/Storage/ResourceLoadStatisticsPersistentStorage.h
+++ b/Source/WebKit/UIProcess/Storage/ResourceLoadStatisticsPersistentStorage.h
@@ -28,6 +28,7 @@
 #include <wtf/Forward.h>
 #include <wtf/MonotonicTime.h>
 #include <wtf/WallTime.h>
+#include <wtf/WeakPtr.h>
 #include <wtf/text/WTFString.h>
 
 namespace WebCore {
@@ -62,6 +63,9 @@ private:
     void excludeFromBackup() const;
     void refreshMemoryStoreFromDisk();
 
+    WeakPtr<ResourceLoadStatisticsPersistentStorage> createWeakPtr() { return m_weakPtrFactory.createWeakPtr(); }
+
+    WeakPtrFactory<ResourceLoadStatisticsPersistentStorage> m_weakPtrFactory;
     WebResourceLoadStatisticsStore& m_memoryStore;
     const String m_storageDirectoryPath;
     std::unique_ptr<WebCore::FileMonitor> m_fileMonitor;