Changeset 285177 in webkit

Timestamp:

Nov 2, 2021, 12:42:06 PM (5 years ago)

Author:

sihui_liu@apple.com

Message:

Terminate unresponsive network process by crashing it
​https://bugs.webkit.org/show_bug.cgi?id=232603

Reviewed by Chris Dumez.

UI process currently kills network process when it does not respond message in some time (network process being
unresponsive). We've found one common case where network process becomes unresponsive is that it is blocked by
some slow operation on the main thread (like file operation in rdar://84511633). To understand what the
operations are and make a fix, we now ask network process to crash itself on IPC thread. In this way, we can get
crash report that includes the call stack of the main thread. To avoid generating too many crash reports, we
only send the crash message to network process when it becomes unresponsive multiple times in a short time
period.

• Platform/IPC/Connection.cpp:

(IPC::terminateDueToIPCTerminateMessage):
(IPC::Connection::processIncomingMessage):

• Scripts/webkit/model.py:
• Scripts/webkit/tests/MessageNames.cpp:

(IPC::description):
(IPC::receiverName):
(IPC::isValidMessageName):

• Scripts/webkit/tests/MessageNames.h:
• UIProcess/Network/NetworkProcessProxy.cpp:

(WebKit::shouldTerminateNetworkProcessBySendingMessage):
(WebKit::NetworkProcessProxy::didBecomeUnresponsive):

Location:

trunk/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• Platform/IPC/Connection.cpp (modified) (3 diffs)
• Scripts/webkit/model.py (modified) (1 diff)
• Scripts/webkit/tests/MessageNames.cpp (modified) (3 diffs)
• Scripts/webkit/tests/MessageNames.h (modified) (1 diff)
• UIProcess/Network/NetworkProcessProxy.cpp (modified) (3 diffs)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2021-11-02  Sihui Liu  <sihui_liu@apple.com>
+
+        Terminate unresponsive network process by crashing it
+        https://bugs.webkit.org/show_bug.cgi?id=232603
+
+        Reviewed by Chris Dumez.
+
+        UI process currently kills network process when it does not respond message in some time (network process being 
+        unresponsive). We've found one common case where network process becomes unresponsive is that it is blocked by 
+        some slow operation on the main thread (like file operation in rdar://84511633). To understand what the 
+        operations are and make a fix, we now ask network process to crash itself on IPC thread. In this way, we can get 
+        crash report that includes the call stack of the main thread. To avoid generating too many crash reports, we 
+        only send the crash message to network process when it becomes unresponsive multiple times in a short time 
+        period.
+
+        * Platform/IPC/Connection.cpp:
+        (IPC::terminateDueToIPCTerminateMessage):
+        (IPC::Connection::processIncomingMessage):
+        * Scripts/webkit/model.py:
+        * Scripts/webkit/tests/MessageNames.cpp:
+        (IPC::description):
+        (IPC::receiverName):
+        (IPC::isValidMessageName):
+        * Scripts/webkit/tests/MessageNames.h:
+        * UIProcess/Network/NetworkProcessProxy.cpp:
+        (WebKit::shouldTerminateNetworkProcessBySendingMessage):
+        (WebKit::NetworkProcessProxy::didBecomeUnresponsive):
+
 2021-11-02  Kate Cheney  <katherine_cheney@apple.com>
 

trunk/Source/WebKit/Platform/IPC/Connection.cpp

@@ -43 +43 @@
 #if PLATFORM(COCOA)
 #include "MachMessage.h"
+#include "WKCrashReporter.h"
 #endif
 
@@ -752 +753 @@
 }
 
+static NEVER_INLINE NO_RETURN_DUE_TO_CRASH void terminateDueToIPCTerminateMessage()
+{
+#if PLATFORM(COCOA)
+    WebKit::logAndSetCrashLogMessage("Receives Terminate message");
+#else
+    WTFLogAlways("Receives Terminate message");
+#endif
+    CRASH();
+}
+
 void Connection::processIncomingMessage(std::unique_ptr<Decoder> message)
 {
@@ -760 +771 @@
         return;
     }
+
+    if (message->messageName() == MessageName::Terminate)
+        return terminateDueToIPCTerminateMessage();
 
     if (!MessageReceiveQueueMap::isValidMessage(*message)) {

trunk/Source/WebKit/Scripts/webkit/model.py

@@ -78 +78 @@
     Message('LegacySessionState', [], [], attributes=[BUILTIN_ATTRIBUTE], condition=None),
     Message('SetStreamDestinationID', [], [], attributes=[BUILTIN_ATTRIBUTE], condition=None),
-    Message('ProcessOutOfStreamMessage', [], [], attributes=[BUILTIN_ATTRIBUTE], condition=None)
+    Message('ProcessOutOfStreamMessage', [], [], attributes=[BUILTIN_ATTRIBUTE], condition=None),
+    Message('Terminate', [], [], attributes=[BUILTIN_ATTRIBUTE], condition=None),
 ], condition=None)
 

trunk/Source/WebKit/Scripts/webkit/tests/MessageNames.cpp

@@ -157 +157 @@
     case MessageName::SyncMessageReply:
         return "SyncMessageReply";
+    case MessageName::Terminate:
+        return "Terminate";
     case MessageName::TestWithSuperclass_TestAsyncMessageReply:
         return "TestWithSuperclass_TestAsyncMessageReply";
@@ -259 +261 @@
     case MessageName::SetStreamDestinationID:
     case MessageName::SyncMessageReply:
+    case MessageName::Terminate:
         return ReceiverName::IPC;
     case MessageName::TestWithSuperclass_TestAsyncMessageReply:
@@ -467 +470 @@
     if (messageName == IPC::MessageName::SyncMessageReply)
         return true;
+    if (messageName == IPC::MessageName::Terminate)
+        return true;
 #if ENABLE(TEST_FEATURE)
     if (messageName == IPC::MessageName::TestWithSuperclass_TestAsyncMessageReply)

trunk/Source/WebKit/Scripts/webkit/tests/MessageNames.h

@@ -108 +108 @@
     , SetStreamDestinationID
     , SyncMessageReply
+    , Terminate
     , TestWithSuperclass_TestAsyncMessageReply
     , TestWithSuperclass_TestAsyncMessageWithConnectionReply

trunk/Source/WebKit/UIProcess/Network/NetworkProcessProxy.cpp

@@ -88 +88 @@
 
 static constexpr Seconds networkProcessResponsivenessTimeout = 6_s;
+static constexpr int unresponsivenessCountLimit = 3;
+static constexpr Seconds unresponsivenessCheckPeriod = 15_s;
 
 static HashSet<NetworkProcessProxy*>& networkProcessesSet()
@@ -94 +96 @@
     static NeverDestroyed<HashSet<NetworkProcessProxy*>> set;
     return set;
+}
+
+static bool shouldTerminateNetworkProcessBySendingMessage()
+{
+    static WallTime unresponsivenessPeriodStartTime = WallTime::now();
+    static int unresponsivenessCountDuringThisPeriod = 0;
+    auto now = WallTime::now();
+
+    if (now - unresponsivenessPeriodStartTime > unresponsivenessCheckPeriod) {
+        unresponsivenessCountDuringThisPeriod = 1;
+        unresponsivenessPeriodStartTime = now;
+        return false;
+    }
+
+    ++unresponsivenessCountDuringThisPeriod;
+    if (unresponsivenessCountDuringThisPeriod >= unresponsivenessCountLimit)
+        return true;
+
+    return false;
 }
 
@@ -129 +150 @@
 {
     RELEASE_LOG_ERROR(Process, "NetworkProcessProxy::didBecomeUnresponsive: NetworkProcess with PID %d became unresponsive, terminating it", processIdentifier());
+
+    // Let network process terminates itself and generate crash report for investigation of hangs.
+    // We currently only do this when network process becomes unresponsive multiple times in a short
+    // time period to avoid generating too many crash reports with same back trace on user's device.
+    if (shouldTerminateNetworkProcessBySendingMessage()) {
+        sendMessage(makeUniqueRef<IPC::Encoder>(IPC::MessageName::Terminate, 0), { });
+        RunLoop::main().dispatchAfter(1_s, [weakThis = WeakPtr { *this }] () mutable {
+            if (weakThis)
+                weakThis->terminate();
+        });
+        return;
+    }
+
     terminate();
 }