Changeset 187129 in webkit

Timestamp:

Jul 21, 2015, 3:33:53 PM (11 years ago)

Author:

dbates@webkit.org

Message:

WTFCrash() in WebKit::WebProcess::networkConnection()
​https://bugs.webkit.org/show_bug.cgi?id=147112
<rdar://problem/18477459>

Reviewed by Gavin Barraclough.

Source/WebKit2:

Fixes an issue where a newly launched network process may be jetsam'd because it has not
taken a process assertion between the time it was launched and the time when a web process
makes use of it.

Initially a network process does not have a process assertion. A process assertion is taken
(if one has not been taken) for the network process when a process assertion is taken for at
least one web process. When the network process crashes a WebProcess may ultimately launch a
new network process in WebProcess::networkConnection(). The new network process may be jetsam'd
immediately when the system is under some measure pressure because it has a low jetsam priority,
0 (since it does not have a process assertion and higher priority implies that a process is less
likely to be jetsam'd). And the logic in WebProcess::networkConnection() explicitly calls
CRASH() if the newly launched network process crashes immediately. Towards preventing the newly
launched network process from being jetsam'd we should obtain a process assertion for it.

• UIProcess/WebProcessPool.cpp:

(WebKit::WebProcessPool::WebProcessPool): Initialize m_didNetworkProcessCrash to false.
(WebKit::WebProcessPool::ensureNetworkProcess): If the network process crashed (m_didNetworkProcessCrash == true)
then tell each process in the pool to reinstate their network activity token for the new network process.
(WebKit::WebProcessPool::networkProcessCrashed): Set m_didNetworkProcessCrash to true when the
network process crashed.

• UIProcess/WebProcessPool.h:
• UIProcess/WebProcessProxy.cpp:

(WebKit::WebProcessProxy::reinstateNetworkProcessAssertionState): Added.
(WebKit::WebProcessProxy::didSetAssertionState): Add assert to ensure we never have both
a background- and foreground- activity token for the network process.

• UIProcess/WebProcessProxy.h:

Source/WTF:

Add explicit boolean conversion function and remove overload of operator! to support
checking whether an activity token is valid more directly than using the overloaded operator!.

• wtf/RefCounter.h:

(WTF::RefCounter::Token::operator bool): Added.
(WTF::RefCounter::Token::operator!): Deleted.

Location:

trunk/Source

Files:

• WTF/ChangeLog (modified) (1 diff)
• WTF/wtf/RefCounter.h (modified) (1 diff)
• WebKit2/ChangeLog (modified) (1 diff)
• WebKit2/UIProcess/WebProcessPool.cpp (modified) (3 diffs)
• WebKit2/UIProcess/WebProcessPool.h (modified) (1 diff)
• WebKit2/UIProcess/WebProcessProxy.cpp (modified) (2 diffs)
• WebKit2/UIProcess/WebProcessProxy.h (modified) (2 diffs)

trunk/Source/WTF/ChangeLog

@@ +1 @@
+2015-07-21  Daniel Bates  <dabates@apple.com>
+
+        WTFCrash() in WebKit::WebProcess::networkConnection()
+        https://bugs.webkit.org/show_bug.cgi?id=147112
+        <rdar://problem/18477459>
+
+        Reviewed by Gavin Barraclough.
+
+        Add explicit boolean conversion function and remove overload of operator! to support
+        checking whether an activity token is valid more directly than using the overloaded operator!.
+
+        * wtf/RefCounter.h:
+        (WTF::RefCounter::Token::operator bool): Added.
+        (WTF::RefCounter::Token::operator!): Deleted.
+
 2015-07-20  Mark Lam  <mark.lam@apple.com>
 

trunk/Source/WTF/wtf/RefCounter.h

@@ -68 +68 @@
         inline Token<T>& operator=(Token<T>&&);
 
-        bool operator!() const { return !m_ptr; }
+        explicit operator bool() const { return m_ptr; }
 
     private:

trunk/Source/WebKit2/ChangeLog

@@ +1 @@
+2015-07-21  Daniel Bates  <dabates@apple.com>
+
+        WTFCrash() in WebKit::WebProcess::networkConnection()
+        https://bugs.webkit.org/show_bug.cgi?id=147112
+        <rdar://problem/18477459>
+
+        Reviewed by Gavin Barraclough.
+
+        Fixes an issue where a newly launched network process may be jetsam'd because it has not
+        taken a process assertion between the time it was launched and the time when a web process
+        makes use of it.
+
+        Initially a network process does not have a process assertion. A process assertion is taken
+        (if one has not been taken) for the network process when a process assertion is taken for at
+        least one web process. When the network process crashes a WebProcess may ultimately launch a
+        new network process in WebProcess::networkConnection(). The new network process may be jetsam'd
+        immediately when the system is under some measure pressure because it has a low jetsam priority,
+        0 (since it does not have a process assertion and higher priority implies that a process is less
+        likely to be jetsam'd). And the logic in WebProcess::networkConnection() explicitly calls
+        CRASH() if the newly launched network process crashes immediately. Towards preventing the newly
+        launched network process from being jetsam'd we should obtain a process assertion for it.
+
+        * UIProcess/WebProcessPool.cpp:
+        (WebKit::WebProcessPool::WebProcessPool): Initialize m_didNetworkProcessCrash to false.
+        (WebKit::WebProcessPool::ensureNetworkProcess): If the network process crashed (m_didNetworkProcessCrash == true)
+        then tell each process in the pool to reinstate their network activity token for the new network process.
+        (WebKit::WebProcessPool::networkProcessCrashed): Set m_didNetworkProcessCrash to true when the
+        network process crashed.
+        * UIProcess/WebProcessPool.h:
+        * UIProcess/WebProcessProxy.cpp:
+        (WebKit::WebProcessProxy::reinstateNetworkProcessAssertionState): Added.
+        (WebKit::WebProcessProxy::didSetAssertionState): Add assert to ensure we never have both
+        a background- and foreground- activity token for the network process.
+        * UIProcess/WebProcessProxy.h:
+
 2015-07-21  Daniel Bates  <dabates@apple.com>
 

trunk/Source/WebKit2/UIProcess/WebProcessPool.cpp

@@ -161 +161 @@
 #if ENABLE(NETWORK_PROCESS)
     , m_canHandleHTTPSServerTrustEvaluation(true)
+    , m_didNetworkProcessCrash(false)
 #endif
 #if USE(SOUP)
@@ -420 +421 @@
 #endif
 
+    if (m_didNetworkProcessCrash) {
+        m_didNetworkProcessCrash = false;
+        for (auto& process : m_processes)
+            process->reinstateNetworkProcessAssertionState(*m_networkProcess);
+    }
+
     return *m_networkProcess;
 }
@@ -427 +434 @@
     ASSERT(m_networkProcess);
     ASSERT(networkProcessProxy == m_networkProcess.get());
+    m_didNetworkProcessCrash = true;
 
     WebContextSupplementMap::const_iterator it = m_supplements.begin();

trunk/Source/WebKit2/UIProcess/WebProcessPool.h

@@ -499 +499 @@
 #if ENABLE(NETWORK_PROCESS)
     bool m_canHandleHTTPSServerTrustEvaluation;
+    bool m_didNetworkProcessCrash;
     RefPtr<NetworkProcessProxy> m_networkProcess;
 #endif

trunk/Source/WebKit2/UIProcess/WebProcessProxy.cpp

@@ -926 +926 @@
 }
 
+#if ENABLE(NETWORK_PROCESS)
+void WebProcessProxy::reinstateNetworkProcessAssertionState(NetworkProcessProxy& newNetworkProcessProxy)
+{
+    ASSERT(!m_backgroundTokenForNetworkProcess || !m_foregroundTokenForNetworkProcess);
+
+    // The network process crashed; take new tokens for the new network process.
+    if (m_backgroundTokenForNetworkProcess)
+        m_backgroundTokenForNetworkProcess = newNetworkProcessProxy.throttler().backgroundActivityToken();
+    else if (m_foregroundTokenForNetworkProcess)
+        m_foregroundTokenForNetworkProcess = newNetworkProcessProxy.throttler().foregroundActivityToken();
+}
+#endif
+
 void WebProcessProxy::didSetAssertionState(AssertionState state)
 {
 #if PLATFORM(IOS) && ENABLE(NETWORK_PROCESS)
+    ASSERT(!m_backgroundTokenForNetworkProcess || !m_foregroundTokenForNetworkProcess);
+
     switch (state) {
     case AssertionState::Suspended:
@@ -951 +966 @@
         break;
     }
+
+    ASSERT(!m_backgroundTokenForNetworkProcess || !m_foregroundTokenForNetworkProcess);
 #else
     UNUSED_PARAM(state);

trunk/Source/WebKit2/UIProcess/WebProcessProxy.h

@@ -60 +60 @@
 
 class DownloadProxyMap;
+class NetworkProcessProxy;
 class WebBackForwardListItem;
 class WebPageGroup;
@@ -151 +152 @@
     ProcessThrottler& throttler() { return m_throttler; }
 
+#if ENABLE(NETWORK_PROCESS)
+    void reinstateNetworkProcessAssertionState(NetworkProcessProxy&);
+#endif
+
 private:
     explicit WebProcessProxy(WebProcessPool&);