Changeset 225253 in webkit

Timestamp:

Nov 28, 2017 6:23:38 PM (6 years ago)

Author:

commit-queue@webkit.org

Message:

NetworkCache::Storage should protect itself when removing operations from its maps
​https://bugs.webkit.org/show_bug.cgi?id=180118

Patch by Youenn Fablet <​youenn@apple.com> on 2017-11-28
Reviewed by Antti Koivisto.

The operations can contain ref to the Storage object and removing them from the map may destroy the Storage object

• NetworkProcess/cache/NetworkCacheStorage.cpp:

(WebKit::NetworkCache::Storage::remove):
(WebKit::NetworkCache::Storage::finishReadOperation):
(WebKit::NetworkCache::Storage::finishWriteOperation):
(WebKit::NetworkCache::Storage::traverse):

Location:

trunk/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• NetworkProcess/cache/NetworkCacheStorage.cpp (modified) (5 diffs)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2017-11-28  Youenn Fablet  <youenn@apple.com>
+
+        NetworkCache::Storage should protect itself when removing operations from its maps
+        https://bugs.webkit.org/show_bug.cgi?id=180118
+
+        Reviewed by Antti Koivisto.
+
+        The operations can contain ref to the Storage object and removing them from the map may destroy the Storage object
+
+        * NetworkProcess/cache/NetworkCacheStorage.cpp:
+        (WebKit::NetworkCache::Storage::remove):
+        (WebKit::NetworkCache::Storage::finishReadOperation):
+        (WebKit::NetworkCache::Storage::finishWriteOperation):
+        (WebKit::NetworkCache::Storage::traverse):
+
 2017-11-28  Youenn Fablet  <youenn@apple.com>
 

trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheStorage.cpp

@@ -562 +562 @@
         return;
 
+    auto protectedThis = makeRef(*this);
+
     // We can't remove the key from the Bloom filter (but some false positives are expected anyway).
     // For simplicity we also don't reduce m_approximateSize on removals.
@@ -568 +570 @@
     removeFromPendingWriteOperations(key);
 
-    serialBackgroundIOQueue().dispatch([this, protectedThis = makeRef(*this), key] () mutable {
+    serialBackgroundIOQueue().dispatch([this, protectedThis = WTFMove(protectedThis), key] () mutable {
         deleteFiles(key);
         RunLoop::main().dispatch([protectedThis = WTFMove(protectedThis)] { });
@@ -666 +668 @@
         else if (!readOperation.isCanceled)
             remove(readOperation.key);
+
+        auto protectedThis = makeRef(*this);
 
         ASSERT(m_activeReadOperations.contains(&readOperation));
@@ -794 +798 @@
     if (--writeOperation.activeCount)
         return;
+
+    auto protectedThis = makeRef(*this);
 
     m_activeWriteOperations.remove(&writeOperation);
@@ -916 +922 @@
         RunLoop::main().dispatch([this, &traverseOperation] {
             traverseOperation.handler(nullptr, { });
+
+            auto protectedThis = makeRef(*this);
+
             m_activeTraverseOperations.remove(&traverseOperation);
         });