Title: [233663] branches/safari-606.1.24-branch/Source/WebKit
Revision
233663
Author
[email protected]
Date
2018-07-09 16:28:51 -0700 (Mon, 09 Jul 2018)

Log Message

Cherry-pick r233601. rdar://problem/41909647

    Add release assertions to try and catch a possible HashMap corruption in NetworkConnectionToWebProcess
    https://bugs.webkit.org/show_bug.cgi?id=187417

    Reviewed by Ryosuke Niwa.

    Add assertions to make sure we:
    - Always use NetworkConnectionToWebProcess::m_networkResourceLoaders from the main thread
    - Never use a 0 identifier as key for NetworkConnectionToWebProcess::m_networkResourceLoaders

    We see crashes (rdar://problem/39265927) that only seem to make sense if this HashMap was
    somehow getting corrupted. Let's try and catch the most common reasons for HashMap corruption.

    * NetworkProcess/NetworkConnectionToWebProcess.cpp:
    (WebKit::NetworkConnectionToWebProcess::didCleanupResourceLoader):
    (WebKit::NetworkConnectionToWebProcess::didReceiveMessage):
    (WebKit::NetworkConnectionToWebProcess::scheduleResourceLoad):
    (WebKit::NetworkConnectionToWebProcess::performSynchronousLoad):
    (WebKit::NetworkConnectionToWebProcess::removeLoadIdentifier):
    (WebKit::NetworkConnectionToWebProcess::setDefersLoading):
    (WebKit::NetworkConnectionToWebProcess::convertMainResourceLoadToDownload):
    * WebProcess/Network/WebResourceLoader.cpp:
    (WebKit::WebResourceLoader::messageSenderDestinationID):

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

Modified Paths

Diff

Modified: branches/safari-606.1.24-branch/Source/WebKit/ChangeLog (233662 => 233663)


--- branches/safari-606.1.24-branch/Source/WebKit/ChangeLog	2018-07-09 23:11:36 UTC (rev 233662)
+++ branches/safari-606.1.24-branch/Source/WebKit/ChangeLog	2018-07-09 23:28:51 UTC (rev 233663)
@@ -1,3 +1,58 @@
+2018-07-09  Babak Shafiei  <[email protected]>
+
+        Cherry-pick r233601. rdar://problem/41909647
+
+    Add release assertions to try and catch a possible HashMap corruption in NetworkConnectionToWebProcess
+    https://bugs.webkit.org/show_bug.cgi?id=187417
+    
+    Reviewed by Ryosuke Niwa.
+    
+    Add assertions to make sure we:
+    - Always use NetworkConnectionToWebProcess::m_networkResourceLoaders from the main thread
+    - Never use a 0 identifier as key for NetworkConnectionToWebProcess::m_networkResourceLoaders
+    
+    We see crashes (rdar://problem/39265927) that only seem to make sense if this HashMap was
+    somehow getting corrupted. Let's try and catch the most common reasons for HashMap corruption.
+    
+    * NetworkProcess/NetworkConnectionToWebProcess.cpp:
+    (WebKit::NetworkConnectionToWebProcess::didCleanupResourceLoader):
+    (WebKit::NetworkConnectionToWebProcess::didReceiveMessage):
+    (WebKit::NetworkConnectionToWebProcess::scheduleResourceLoad):
+    (WebKit::NetworkConnectionToWebProcess::performSynchronousLoad):
+    (WebKit::NetworkConnectionToWebProcess::removeLoadIdentifier):
+    (WebKit::NetworkConnectionToWebProcess::setDefersLoading):
+    (WebKit::NetworkConnectionToWebProcess::convertMainResourceLoadToDownload):
+    * WebProcess/Network/WebResourceLoader.cpp:
+    (WebKit::WebResourceLoader::messageSenderDestinationID):
+    
+    
+    git-svn-id: https://svn.webkit.org/repository/webkit/trunk@233601 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+    2018-07-06  Chris Dumez  <[email protected]>
+
+            Add release assertions to try and catch a possible HashMap corruption in NetworkConnectionToWebProcess
+            https://bugs.webkit.org/show_bug.cgi?id=187417
+
+            Reviewed by Ryosuke Niwa.
+
+            Add assertions to make sure we:
+            - Always use NetworkConnectionToWebProcess::m_networkResourceLoaders from the main thread
+            - Never use a 0 identifier as key for NetworkConnectionToWebProcess::m_networkResourceLoaders
+
+            We see crashes (rdar://problem/39265927) that only seem to make sense if this HashMap was
+            somehow getting corrupted. Let's try and catch the most common reasons for HashMap corruption.
+
+            * NetworkProcess/NetworkConnectionToWebProcess.cpp:
+            (WebKit::NetworkConnectionToWebProcess::didCleanupResourceLoader):
+            (WebKit::NetworkConnectionToWebProcess::didReceiveMessage):
+            (WebKit::NetworkConnectionToWebProcess::scheduleResourceLoad):
+            (WebKit::NetworkConnectionToWebProcess::performSynchronousLoad):
+            (WebKit::NetworkConnectionToWebProcess::removeLoadIdentifier):
+            (WebKit::NetworkConnectionToWebProcess::setDefersLoading):
+            (WebKit::NetworkConnectionToWebProcess::convertMainResourceLoadToDownload):
+            * WebProcess/Network/WebResourceLoader.cpp:
+            (WebKit::WebResourceLoader::messageSenderDestinationID):
+
 2018-07-07  Babak Shafiei  <[email protected]>
 
         Cherry-pick r233616. rdar://problem/41931915

Modified: branches/safari-606.1.24-branch/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp (233662 => 233663)


--- branches/safari-606.1.24-branch/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp	2018-07-09 23:11:36 UTC (rev 233662)
+++ branches/safari-606.1.24-branch/Source/WebKit/NetworkProcess/NetworkConnectionToWebProcess.cpp	2018-07-09 23:28:51 UTC (rev 233663)
@@ -88,6 +88,8 @@
 
 void NetworkConnectionToWebProcess::didCleanupResourceLoader(NetworkResourceLoader& loader)
 {
+    RELEASE_ASSERT(loader.identifier());
+    RELEASE_ASSERT(RunLoop::isMain());
     ASSERT(m_networkResourceLoaders.get(loader.identifier()) == &loader);
 
     m_networkResourceLoaders.remove(loader.identifier());
@@ -101,9 +103,13 @@
     }
 
     if (decoder.messageReceiverName() == Messages::NetworkResourceLoader::messageReceiverName()) {
+        RELEASE_ASSERT(RunLoop::isMain());
+        RELEASE_ASSERT(decoder.destinationID());
         auto loaderIterator = m_networkResourceLoaders.find(decoder.destinationID());
-        if (loaderIterator != m_networkResourceLoaders.end())
+        if (loaderIterator != m_networkResourceLoaders.end()) {
+            RELEASE_ASSERT(loaderIterator->value);
             loaderIterator->value->didReceiveNetworkResourceLoaderMessage(connection, decoder);
+        }
         return;
     }
 
@@ -238,6 +244,8 @@
 void NetworkConnectionToWebProcess::scheduleResourceLoad(NetworkResourceLoadParameters&& loadParameters)
 {
     auto identifier = loadParameters.identifier;
+    RELEASE_ASSERT(identifier);
+    RELEASE_ASSERT(RunLoop::isMain());
     ASSERT(!m_networkResourceLoaders.contains(identifier));
 
     auto loader = NetworkResourceLoader::create(WTFMove(loadParameters), *this);
@@ -248,6 +256,8 @@
 void NetworkConnectionToWebProcess::performSynchronousLoad(NetworkResourceLoadParameters&& loadParameters, Messages::NetworkConnectionToWebProcess::PerformSynchronousLoad::DelayedReply&& reply)
 {
     auto identifier = loadParameters.identifier;
+    RELEASE_ASSERT(identifier);
+    RELEASE_ASSERT(RunLoop::isMain());
     ASSERT(!m_networkResourceLoaders.contains(identifier));
 
     auto loader = NetworkResourceLoader::create(WTFMove(loadParameters), *this, WTFMove(reply));
@@ -280,6 +290,9 @@
 
 void NetworkConnectionToWebProcess::removeLoadIdentifier(ResourceLoadIdentifier identifier)
 {
+    RELEASE_ASSERT(identifier);
+    RELEASE_ASSERT(RunLoop::isMain());
+
     RefPtr<NetworkResourceLoader> loader = m_networkResourceLoaders.get(identifier);
 
     // It's possible we have no loader for this identifier if the NetworkProcess crashed and this was a respawned NetworkProcess.
@@ -299,6 +312,9 @@
 
 void NetworkConnectionToWebProcess::setDefersLoading(ResourceLoadIdentifier identifier, bool defers)
 {
+    RELEASE_ASSERT(identifier);
+    RELEASE_ASSERT(RunLoop::isMain());
+
     RefPtr<NetworkResourceLoader> loader = m_networkResourceLoaders.get(identifier);
     if (!loader)
         return;
@@ -352,6 +368,8 @@
 
 void NetworkConnectionToWebProcess::convertMainResourceLoadToDownload(PAL::SessionID sessionID, uint64_t mainResourceLoadIdentifier, DownloadID downloadID, const ResourceRequest& request, const ResourceResponse& response)
 {
+    RELEASE_ASSERT(RunLoop::isMain());
+
     auto& networkProcess = NetworkProcess::singleton();
     // In case a response is served from service worker, we do not have yet the ability to convert the load.
     if (!mainResourceLoadIdentifier || response.source() == ResourceResponse::Source::ServiceWorker) {

Modified: branches/safari-606.1.24-branch/Source/WebKit/WebProcess/Network/WebResourceLoader.cpp (233662 => 233663)


--- branches/safari-606.1.24-branch/Source/WebKit/WebProcess/Network/WebResourceLoader.cpp	2018-07-09 23:11:36 UTC (rev 233662)
+++ branches/safari-606.1.24-branch/Source/WebKit/WebProcess/Network/WebResourceLoader.cpp	2018-07-09 23:28:51 UTC (rev 233663)
@@ -73,11 +73,14 @@
 
 uint64_t WebResourceLoader::messageSenderDestinationID()
 {
+    RELEASE_ASSERT(RunLoop::isMain());
+    RELEASE_ASSERT(m_coreLoader->identifier());
     return m_coreLoader->identifier();
 }
 
 void WebResourceLoader::detachFromCoreLoader()
 {
+    RELEASE_ASSERT(RunLoop::isMain());
     m_coreLoader = nullptr;
 }
 
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to