Title: [263852] trunk/Source/WebKit
Revision
263852
Author
cdu...@apple.com
Date
2020-07-02 11:45:34 -0700 (Thu, 02 Jul 2020)

Log Message

Crash under WebKit::NetworkProcessProxy::updateProcessAssertion()
https://bugs.webkit.org/show_bug.cgi?id=213891
<rdar://problem/65017909>

Reviewed by Alex Christensen.

The crash was due to NetworkProcessProxy::updateProcessAssertion() re-entering while
in the middle of the `m_activityFromWebProcesses = nullptr;` assignment. Calling
the ProcessThrottler::BackgroundActivity destructor, could cause updateProcessAssertion()
to get called again, in which case we may dereference m_activityFromWebProcesses and
crash. To address the issue, use `std::exchange(m_activityFromWebProcesses, nullptr);`
instead, so that m_activityFromWebProcesses becomes null BEFORE the BackgroundActivity
destructor gets called. updateProcessAssertion() will still re-enter but
m_activityFromWebProcesses will be nullptr and updateProcessAssertion() will do the
right thing.

* UIProcess/Network/NetworkProcessProxy.cpp:
(WebKit::NetworkProcessProxy::updateProcessAssertion):

Modified Paths

Diff

Modified: trunk/Source/WebKit/ChangeLog (263851 => 263852)


--- trunk/Source/WebKit/ChangeLog	2020-07-02 17:56:10 UTC (rev 263851)
+++ trunk/Source/WebKit/ChangeLog	2020-07-02 18:45:34 UTC (rev 263852)
@@ -1,3 +1,24 @@
+2020-07-02  Chris Dumez  <cdu...@apple.com>
+
+        Crash under WebKit::NetworkProcessProxy::updateProcessAssertion()
+        https://bugs.webkit.org/show_bug.cgi?id=213891
+        <rdar://problem/65017909>
+
+        Reviewed by Alex Christensen.
+
+        The crash was due to NetworkProcessProxy::updateProcessAssertion() re-entering while
+        in the middle of the `m_activityFromWebProcesses = nullptr;` assignment. Calling
+        the ProcessThrottler::BackgroundActivity destructor, could cause updateProcessAssertion()
+        to get called again, in which case we may dereference m_activityFromWebProcesses and
+        crash. To address the issue, use `std::exchange(m_activityFromWebProcesses, nullptr);`
+        instead, so that m_activityFromWebProcesses becomes null BEFORE the BackgroundActivity
+        destructor gets called. updateProcessAssertion() will still re-enter but
+        m_activityFromWebProcesses will be nullptr and updateProcessAssertion() will do the
+        right thing.
+
+        * UIProcess/Network/NetworkProcessProxy.cpp:
+        (WebKit::NetworkProcessProxy::updateProcessAssertion):
+
 2020-07-02  Carlos Garcia Campos  <cgar...@igalia.com>
 
         Unreviewed. Fix GTK4 build

Modified: trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp (263851 => 263852)


--- trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp	2020-07-02 17:56:10 UTC (rev 263851)
+++ trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp	2020-07-02 18:45:34 UTC (rev 263852)
@@ -1525,7 +1525,10 @@
         }
         return;
     }
-    m_activityFromWebProcesses = nullptr;
+    // Use std::exchange() instead of a simple nullptr assignment to avoid re-entering this
+    // function during the destructor of the ProcessThrottler activity, before setting
+    // m_activityFromWebProcesses.
+    std::exchange(m_activityFromWebProcesses, nullptr);
 }
 
 void NetworkProcessProxy::resetQuota(PAL::SessionID sessionID, CompletionHandler<void()>&& completionHandler)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to