Changeset 263996 in webkit

Timestamp:

Jul 6, 2020, 4:30:05 PM (5 years ago)

Author:

Chris Dumez

Message:

Regression(r249303) Crash under NetworkLoad::NetworkLoad()
​https://bugs.webkit.org/show_bug.cgi?id=214008
<rdar://problem/64853936>

Reviewed by Alex Christensen.

• NetworkProcess/cache/NetworkCacheSpeculativeLoad.cpp:

(WebKit::NetworkCache::SpeculativeLoad::SpeculativeLoad):
Do some hardening and fail the SpeculativeLoad if the network session is null, instead of dereferencing
the network session unconditionally. The NetworkCache owns the NetworkCacheSpeculativeLoadManager and
the NetworkCache is RefCounted so it may outlive its NetworkSession in theory and schedule speculative
loads for a session that was just destroyed.

• NetworkProcess/cache/NetworkCacheSpeculativeLoadManager.cpp:

(WebKit::NetworkCache::SpeculativeLoadManager::registerLoad):
(WebKit::NetworkCache::SpeculativeLoadManager::preloadEntry):

• NetworkProcess/cache/NetworkCacheSpeculativeLoadManager.h:

Capture weakThis in a few lambda and check it when the lambda gets called. It looked unsafe so I
decided to do some hardening.

Location:

trunk/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• NetworkProcess/cache/NetworkCacheSpeculativeLoad.cpp (modified) (2 diffs)
• NetworkProcess/cache/NetworkCacheSpeculativeLoadManager.cpp (modified) (2 diffs)
• NetworkProcess/cache/NetworkCacheSpeculativeLoadManager.h (modified) (2 diffs)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2020-07-06  Chris Dumez  <cdumez@apple.com>
+
+        Regression(r249303) Crash under NetworkLoad::NetworkLoad()
+        https://bugs.webkit.org/show_bug.cgi?id=214008
+        <rdar://problem/64853936>
+
+        Reviewed by Alex Christensen.
+
+        * NetworkProcess/cache/NetworkCacheSpeculativeLoad.cpp:
+        (WebKit::NetworkCache::SpeculativeLoad::SpeculativeLoad):
+        Do some hardening and fail the SpeculativeLoad if the network session is null, instead of dereferencing
+        the network session unconditionally. The NetworkCache owns the NetworkCacheSpeculativeLoadManager and
+        the NetworkCache is RefCounted so it may outlive its NetworkSession in theory and schedule speculative
+        loads for a session that was just destroyed.
+
+        * NetworkProcess/cache/NetworkCacheSpeculativeLoadManager.cpp:
+        (WebKit::NetworkCache::SpeculativeLoadManager::registerLoad):
+        (WebKit::NetworkCache::SpeculativeLoadManager::preloadEntry):
+        * NetworkProcess/cache/NetworkCacheSpeculativeLoadManager.h:
+        Capture weakThis in a few lambda and check it when the lambda gets called. It looked unsafe so I
+        decided to do some hardening.
+
 2020-07-06  Peng Liu  <peng.liu6@apple.com>
 

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

@@ -52 +52 @@
     ASSERT(!m_cacheEntry || m_cacheEntry->needsValidation());
 
+    auto* networkSession = m_cache->networkProcess().networkSession(m_cache->sessionID());
+    if (!networkSession) {
+        RunLoop::main().dispatch([completionHandler = WTFMove(m_completionHandler)]() mutable {
+            completionHandler(nullptr);
+        });
+        return;
+    }
+
     NetworkLoadParameters parameters;
     parameters.webPageProxyID = globalFrameID.webPageProxyID;
@@ -61 +69 @@
     parameters.request = m_originalRequest;
     parameters.isNavigatingToAppBoundDomain = isNavigatingToAppBoundDomain;
-    m_networkLoad = makeUnique<NetworkLoad>(*this, nullptr, WTFMove(parameters), *cache.networkProcess().networkSession(cache.sessionID()));
+    m_networkLoad = makeUnique<NetworkLoad>(*this, nullptr, WTFMove(parameters), *networkSession);
 }
 

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

@@ -368 +368 @@
 
         // Retrieve the subresources entry if it exists to start speculative revalidation and to update it.
-        retrieveSubresourcesEntry(resourceKey, [this, frameID, pendingFrameLoad = WTFMove(pendingFrameLoad), isNavigatingToAppBoundDomain](std::unique_ptr<SubresourcesEntry> entry) {
+        retrieveSubresourcesEntry(resourceKey, [this, weakThis = makeWeakPtr(*this), frameID, pendingFrameLoad = WTFMove(pendingFrameLoad), isNavigatingToAppBoundDomain](std::unique_ptr<SubresourcesEntry> entry) {
+            if (!weakThis)
+                return;
+
             if (entry)
                 startSpeculativeRevalidation(frameID, *entry, isNavigatingToAppBoundDomain);
@@ -560 +563 @@
     m_pendingPreloads.add(key, nullptr);
     
-    retrieveEntryFromStorage(subresourceInfo, [this, key, subresourceInfo, frameID, isNavigatingToAppBoundDomain](std::unique_ptr<Entry> entry) {
+    retrieveEntryFromStorage(subresourceInfo, [this, weakThis = makeWeakPtr(*this), key, subresourceInfo, frameID, isNavigatingToAppBoundDomain](std::unique_ptr<Entry> entry) {
+        if (!weakThis)
+            return;
+
         ASSERT(!m_pendingPreloads.get(key));
         bool removed = m_pendingPreloads.remove(key);

trunk/Source/WebKit/NetworkProcess/cache/NetworkCacheSpeculativeLoadManager.h

@@ -34 +34 @@
 #include <wtf/HashMap.h>
 #include <wtf/Vector.h>
+#include <wtf/WeakPtr.h>
 
 namespace WebKit {
@@ -44 +45 @@
 class SubresourcesEntry;
 
-class SpeculativeLoadManager {
+class SpeculativeLoadManager : public CanMakeWeakPtr<SpeculativeLoadManager> {
     WTF_MAKE_FAST_ALLOCATED;
 public: