Modified: trunk/Source/WebKit/ChangeLog (232946 => 232947)
--- trunk/Source/WebKit/ChangeLog 2018-06-18 21:20:47 UTC (rev 232946)
+++ trunk/Source/WebKit/ChangeLog 2018-06-18 21:45:23 UTC (rev 232947)
@@ -1,3 +1,38 @@
+2018-06-18 Chris Dumez <[email protected]>
+
+ Implement IPC throttling to keep the main thread responsive when a process misbehaves
+ https://bugs.webkit.org/show_bug.cgi?id=186607
+
+ Reviewed by Geoffrey Garen.
+
+ 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.
+
+ * 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-18 Jiewen Tan <[email protected]>
Add a graceful exit for AuthenticationManager::initializeConnection
Modified: trunk/Source/WebKit/Platform/IPC/Connection.cpp (232946 => 232947)
--- trunk/Source/WebKit/Platform/IPC/Connection.cpp 2018-06-18 21:20:47 UTC (rev 232946)
+++ trunk/Source/WebKit/Platform/IPC/Connection.cpp 2018-06-18 21:45:23 UTC (rev 232947)
@@ -44,6 +44,11 @@
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;
Function<void (std::unique_ptr<Decoder>)> handler;
@@ -239,6 +244,7 @@
, m_inDispatchMessageCount(0)
, m_inDispatchMessageMarkedDispatchWhenWaitingForSyncReplyCount(0)
, m_didReceiveInvalidMessage(false)
+ , m_incomingMessagesThrottler(*this, &Connection::dispatchIncomingMessages)
, m_waitingForMessage(nullptr)
, m_shouldWaitForSyncReplies(true)
{
@@ -893,11 +899,28 @@
{
{
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 %lu 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_incomingMessages.size() != 1)
+ return;
}
RunLoop::main().dispatch([protectedThis = makeRef(*this)]() mutable {
- protectedThis->dispatchOneMessage();
+ protectedThis->dispatchIncomingMessages();
});
}
@@ -949,10 +972,52 @@
m_didReceiveInvalidMessage = oldDidReceiveInvalidMessage;
}
-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::dispatchIncomingMessages()
+{
+ ASSERT(RunLoop::isMain());
+
std::unique_ptr<Decoder> message;
+ size_t messagesToProcess = 0;
{
std::lock_guard<Lock> lock(m_incomingMessagesMutex);
if (m_incomingMessages.isEmpty())
@@ -959,9 +1024,38 @@
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 %lu pending incoming messages, will only process %lu before yielding)", this, m_incomingMessages.size(), messagesToProcess);
+ 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());
+ }
}
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));
+ }
+
+ {
+ std::lock_guard<Lock> lock(m_incomingMessagesMutex);
+ if (m_incomingMessages.isEmpty())
+ return;
+ }
+
+ m_incomingMessagesThrottler.scheduleMessagesDispatch();
}
void Connection::wakeUpRunLoop()
Modified: trunk/Source/WebKit/Platform/IPC/Connection.h (232946 => 232947)
--- trunk/Source/WebKit/Platform/IPC/Connection.h 2018-06-18 21:20:47 UTC (rev 232946)
+++ trunk/Source/WebKit/Platform/IPC/Connection.h 2018-06-18 21:45:23 UTC (rev 232947)
@@ -40,6 +40,7 @@
#include <wtf/HashMap.h>
#include <wtf/Lock.h>
#include <wtf/OptionSet.h>
+#include <wtf/RunLoop.h>
#include <wtf/WorkQueue.h>
#include <wtf/text/CString.h>
@@ -231,7 +232,7 @@
void connectionDidClose();
// Called on the listener thread.
- void dispatchOneMessage();
+ void dispatchIncomingMessages();
void dispatchMessage(std::unique_ptr<Decoder>);
void dispatchMessage(Decoder&);
void dispatchSyncMessage(Decoder&);
@@ -240,6 +241,7 @@
// Can be called on any thread.
void enqueueIncomingMessage(std::unique_ptr<Decoder>);
+ size_t incomingMessagesDispatchingBatchSize() const;
void willSendSyncMessage(OptionSet<SendSyncOption>);
void didReceiveSyncReply(OptionSet<SendSyncOption>);
@@ -250,6 +252,21 @@
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;
bool m_isServer;
std::atomic<bool> m_isValid { true };
@@ -275,6 +292,7 @@
// Incoming messages.
Lock m_incomingMessagesMutex;
Deque<std::unique_ptr<Decoder>> m_incomingMessages;
+ MessagesThrottler m_incomingMessagesThrottler;
// Outgoing messages.
Lock m_outgoingMessagesMutex;
@@ -339,6 +357,7 @@
bool m_isInitializingSendSource { false };
OSObjectPtr<xpc_connection_t> m_xpcConnection;
+ bool m_wasKilled { false };
#elif OS(WINDOWS)
// Called on the connection queue.
void readEventHandler();
Modified: trunk/Source/WebKit/Platform/IPC/mac/ConnectionMac.mm (232946 => 232947)
--- trunk/Source/WebKit/Platform/IPC/mac/ConnectionMac.mm 2018-06-18 21:20:47 UTC (rev 232946)
+++ trunk/Source/WebKit/Platform/IPC/mac/ConnectionMac.mm 2018-06-18 21:45:23 UTC (rev 232947)
@@ -623,6 +623,7 @@
{
if (m_xpcConnection) {
xpc_connection_kill(m_xpcConnection.get(), SIGKILL);
+ m_wasKilled = true;
return true;
}
