Changeset 233111 in webkit

Timestamp:

Jun 22, 2018, 5:09:37 PM (8 years ago)

Author:

Chris Dumez

Message:

Implement IPC throttling to keep the main thread responsive when a process misbehaves
​https://bugs.webkit.org/show_bug.cgi?id=186607
<rdar://problem/41073205>

Reviewed by Geoff Garen and Brady Eidson.

Implement IPC throttling to keep the main thread responsive when a process misbehaves.
Instead of doing one main runloop dispatch per incoming message, we now do a single
runloop dispatch and process incoming messages in batch. We put a limit on the number
of messages to be processed in a batch (600). If the queue is larger that this limit,
we'll schedule a 0-timer to process remaining messages, giving the main runloop a chance
to process other events. Additionally, if an IPC connection keeps hitting this maximum
batch size limit, we implement back off and we'll further decrease the number of messages
we process in each batch (going as low as 60). This keeps Safari responsive enough to
allow the user to close the bad tab (even on older devices such as iPhone 5s).

Finally, if the incoming message queue becomes too large (50000), we go one step further
and kill the IPC connection in order to maintain performance / battery life.

Every time we apply throttling or terminate a connection due to throttling, we do a
RELEASE_LOG_ERROR() with useful information in order to help diagnose potential issues
in the future.

For now, incoming IPC messages throttling is only enabled on the UIProcess' connections
to the WebProcesses.

• Platform/IPC/Connection.cpp:

(IPC::Connection::Connection):
(IPC::Connection::enqueueIncomingMessage):
(IPC::Connection::MessagesThrottler::MessagesThrottler):
(IPC::Connection::MessagesThrottler::scheduleMessagesDispatch):
(IPC::Connection::MessagesThrottler::numberOfMessagesToProcess):
(IPC::Connection::dispatchIncomingMessages):

• Platform/IPC/Connection.h:
• Platform/IPC/mac/ConnectionMac.mm:

(IPC::Connection::kill):

Location:

trunk/Source/WebKit

Files:

• ChangeLog (modified) (1 diff)
• Platform/IPC/Connection.cpp (modified) (5 diffs)
• Platform/IPC/Connection.h (modified) (7 diffs)
• Platform/IPC/mac/ConnectionMac.mm (modified) (1 diff)
• UIProcess/WebProcessProxy.cpp (modified) (1 diff)

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+2018-06-22  Chris Dumez  <cdumez@apple.com>
+
+        Implement IPC throttling to keep the main thread responsive when a process misbehaves
+        https://bugs.webkit.org/show_bug.cgi?id=186607
+        <rdar://problem/41073205>
+
+        Reviewed by Geoff Garen and Brady Eidson.
+
+        Implement IPC throttling to keep the main thread responsive when a process misbehaves.
+        Instead of doing one main runloop dispatch per incoming message, we now do a single
+        runloop dispatch and process incoming messages in batch. We put a limit on the number
+        of messages to be processed in a batch (600). If the queue is larger that this limit,
+        we'll schedule a 0-timer to process remaining messages, giving the main runloop a chance
+        to process other events. Additionally, if an IPC connection keeps hitting this maximum
+        batch size limit, we implement back off and we'll further decrease the number of messages
+        we process in each batch (going as low as 60). This keeps Safari responsive enough to
+        allow the user to close the bad tab (even on older devices such as iPhone 5s).
+
+        Finally, if the incoming message queue becomes too large (50000), we go one step further
+        and kill the IPC connection in order to maintain performance / battery life.
+
+        Every time we apply throttling or terminate a connection due to throttling, we do a
+        RELEASE_LOG_ERROR() with useful information in order to help diagnose potential issues
+        in the future.
+
+        For now, incoming IPC messages throttling is only enabled on the UIProcess' connections
+        to the WebProcesses.
+
+        * Platform/IPC/Connection.cpp:
+        (IPC::Connection::Connection):
+        (IPC::Connection::enqueueIncomingMessage):
+        (IPC::Connection::MessagesThrottler::MessagesThrottler):
+        (IPC::Connection::MessagesThrottler::scheduleMessagesDispatch):
+        (IPC::Connection::MessagesThrottler::numberOfMessagesToProcess):
+        (IPC::Connection::dispatchIncomingMessages):
+        * Platform/IPC/Connection.h:
+        * Platform/IPC/mac/ConnectionMac.mm:
+        (IPC::Connection::kill):
+
 2018-06-22  Sihui Liu  <sihui_liu@apple.com>
 

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

@@ -45 +45 @@
 namespace IPC {
 
+#if PLATFORM(COCOA)
+// The IPC connection gets killed if the incoming message queue reaches 50000 messages before the main thread has a chance to dispatch them.
+const size_t maxPendingIncomingMessagesKillingThreshold { 50000 };
+#endif
+
 struct Connection::ReplyHandler {
     RefPtr<FunctionDispatcher> dispatcher;
@@ -755 +760 @@
 }
 
+void Connection::enableIncomingMessagesThrottling()
+{
+    if (m_incomingMessagesThrottler)
+        return;
+
+    m_incomingMessagesThrottler = std::make_unique<MessagesThrottler>(*this, &Connection::dispatchIncomingMessages);
+}
+
 void Connection::postConnectionDidCloseOnConnectionWorkQueue()
 {
@@ -894 +907 @@
     {
         std::lock_guard<Lock> lock(m_incomingMessagesMutex);
+
+#if PLATFORM(COCOA)
+        if (m_wasKilled)
+            return;
+
+        if (m_incomingMessages.size() >= maxPendingIncomingMessagesKillingThreshold) {
+            if (kill()) {
+                RELEASE_LOG_ERROR(IPC, "%p - Connection::enqueueIncomingMessage: Over %zu incoming messages have been queued without the main thread processing them, killing the connection as the remote process seems to be misbehaving", this, maxPendingIncomingMessagesKillingThreshold);
+                m_incomingMessages.clear();
+            }
+            return;
+        }
+#endif
+
         m_incomingMessages.append(WTFMove(incomingMessage));
+
+        if (m_incomingMessagesThrottler && m_incomingMessages.size() != 1)
+            return;
     }
 
     RunLoop::main().dispatch([protectedThis = makeRef(*this)]() mutable {
-        protectedThis->dispatchOneMessage();
+        if (protectedThis->m_incomingMessagesThrottler)
+            protectedThis->dispatchIncomingMessages();
+        else
+            protectedThis->dispatchOneIncomingMessage();
     });
 }
@@ -950 +983 @@
 }
 
-void Connection::dispatchOneMessage()
+Connection::MessagesThrottler::MessagesThrottler(Connection& connection, DispatchMessagesFunction dispatchMessages)
+    : m_dispatchMessagesTimer(RunLoop::main(), &connection, dispatchMessages)
+    , m_connection(connection)
+    , m_dispatchMessages(dispatchMessages)
+{
+    ASSERT(RunLoop::isMain());
+}
+
+void Connection::MessagesThrottler::scheduleMessagesDispatch()
+{
+    ASSERT(RunLoop::isMain());
+
+    if (m_throttlingLevel) {
+        m_dispatchMessagesTimer.startOneShot(0_s);
+        return;
+    }
+    RunLoop::main().dispatch([this, protectedConnection = makeRefPtr(&m_connection)]() mutable {
+        (protectedConnection.get()->*m_dispatchMessages)();
+    });
+}
+
+size_t Connection::MessagesThrottler::numberOfMessagesToProcess(size_t totalMessages)
+{
+    ASSERT(RunLoop::isMain());
+
+    // Never dispatch more than 600 messages without returning to the run loop, we can go as low as 60 with maximum throttling level.
+    static const size_t maxIncomingMessagesDispatchingBatchSize { 600 };
+    static const unsigned maxThrottlingLevel = 9;
+
+    size_t batchSize = maxIncomingMessagesDispatchingBatchSize / (m_throttlingLevel + 1);
+
+    if (totalMessages > maxIncomingMessagesDispatchingBatchSize)
+        m_throttlingLevel = std::min(m_throttlingLevel + 1, maxThrottlingLevel);
+    else if (m_throttlingLevel)
+        --m_throttlingLevel;
+
+    return std::min(totalMessages, batchSize);
+}
+
+void Connection::dispatchOneIncomingMessage()
 {
     std::unique_ptr<Decoder> message;
-
     {
         std::lock_guard<Lock> lock(m_incomingMessagesMutex);
@@ -965 +1036 @@
 }
 
+void Connection::dispatchIncomingMessages()
+{
+    ASSERT(RunLoop::isMain());
+
+    std::unique_ptr<Decoder> message;
+
+    size_t messagesToProcess = 0;
+    {
+        std::lock_guard<Lock> lock(m_incomingMessagesMutex);
+        if (m_incomingMessages.isEmpty())
+            return;
+
+        message = m_incomingMessages.takeFirst();
+
+        // Incoming messages may get adding to the queue by the IPC thread while we're dispatching the messages below.
+        // To make sure dispatchIncomingMessages() yields, we only ever process messages that were in the queue when
+        // dispatchIncomingMessages() was called. Additionally, the MessageThrottler may further cap the number of
+        // messages to process to make sure we give the main run loop a chance to process other events.
+        messagesToProcess = m_incomingMessagesThrottler->numberOfMessagesToProcess(m_incomingMessages.size());
+        if (messagesToProcess < m_incomingMessages.size()) {
+            RELEASE_LOG_ERROR(IPC, "%p - Connection::dispatchIncomingMessages: IPC throttling was triggered (has %zu pending incoming messages, will only process %zu before yielding)", this, m_incomingMessages.size(), messagesToProcess);
+#if PLATFORM(COCOA)
+            RELEASE_LOG_ERROR(IPC, "%p - Connection::dispatchIncomingMessages: first IPC message in queue is %{public}s::%{public}s", this, message->messageReceiverName().toString().data(), message->messageName().toString().data());
+#endif
+        }
+
+        // Re-schedule ourselves *before* we dispatch the messages because we want to process follow-up messages if the client
+        // spins a nested run loop while we're dispatching a message. Note that this means we can re-enter this method.
+        if (!m_incomingMessages.isEmpty())
+            m_incomingMessagesThrottler->scheduleMessagesDispatch();
+    }
+
+    dispatchMessage(WTFMove(message));
+
+    for (size_t i = 1; i < messagesToProcess; ++i) {
+        {
+            std::lock_guard<Lock> lock(m_incomingMessagesMutex);
+            if (m_incomingMessages.isEmpty())
+                return;
+
+            message = m_incomingMessages.takeFirst();
+        }
+        dispatchMessage(WTFMove(message));
+    }
+}
+
 void Connection::wakeUpRunLoop()
 {

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

@@ -41 +41 @@
 #include <wtf/Lock.h>
 #include <wtf/OptionSet.h>
+#include <wtf/RunLoop.h>
 #include <wtf/WorkQueue.h>
 #include <wtf/text/CString.h>
@@ -210 +211 @@
     void ignoreTimeoutsForTesting() { m_ignoreTimeoutsForTesting = true; }
 
+    void enableIncomingMessagesThrottling();
+
 private:
     Connection(Identifier, bool isServer, Client&);
@@ -232 +235 @@
     
     // Called on the listener thread.
-    void dispatchOneMessage();
+    void dispatchOneIncomingMessage();
+    void dispatchIncomingMessages();
     void dispatchMessage(std::unique_ptr<Decoder>);
     void dispatchMessage(Decoder&);
@@ -241 +245 @@
     // Can be called on any thread.
     void enqueueIncomingMessage(std::unique_ptr<Decoder>);
+    size_t incomingMessagesDispatchingBatchSize() const;
 
     void willSendSyncMessage(OptionSet<SendSyncOption>);
@@ -250 +255 @@
     bool sendMessage(std::unique_ptr<MachMessage>);
 #endif
+
+    class MessagesThrottler {
+    public:
+        typedef void (Connection::*DispatchMessagesFunction)();
+        MessagesThrottler(Connection&, DispatchMessagesFunction);
+
+        size_t numberOfMessagesToProcess(size_t totalMessages);
+        void scheduleMessagesDispatch();
+
+    private:
+        RunLoop::Timer<Connection> m_dispatchMessagesTimer;
+        Connection& m_connection;
+        DispatchMessagesFunction m_dispatchMessages;
+        unsigned m_throttlingLevel { 0 };
+    };
 
     Client& m_client;
@@ -276 +296 @@
     Lock m_incomingMessagesMutex;
     Deque<std::unique_ptr<Decoder>> m_incomingMessages;
+    std::unique_ptr<MessagesThrottler> m_incomingMessagesThrottler;
 
     // Outgoing messages.
@@ -340 +361 @@
 
     OSObjectPtr<xpc_connection_t> m_xpcConnection;
+    bool m_wasKilled { false };
 #elif OS(WINDOWS)
     // Called on the connection queue.

trunk/Source/WebKit/Platform/IPC/mac/ConnectionMac.mm

@@ -624 +624 @@
     if (m_xpcConnection) {
         xpc_connection_kill(m_xpcConnection.get(), SIGKILL);
+        m_wasKilled = true;
         return true;
     }

trunk/Source/WebKit/UIProcess/WebProcessProxy.cpp

@@ -186 +186 @@
     ASSERT(this->connection() == &connection);
 
+    // Throttling IPC messages coming from the WebProcesses so that the UIProcess stays responsive, even
+    // if one of the WebProcesses misbehaves.
+    connection.enableIncomingMessagesThrottling();
+
 #if ENABLE(SEC_ITEM_SHIM)
     SecItemShimProxy::singleton().initializeConnection(connection);