Unreviewed, rolling out r232995.
Seems to have caused flakiness
Reverted changeset:
"Implement IPC throttling to keep the main thread responsive
when a process misbehaves"
https://bugs.webkit.org/show_bug.cgi?id=186607
https://trac.webkit.org/changeset/232995
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@233068 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
index ffc8db0..02a8eef 100644
--- a/Source/WebKit/ChangeLog
+++ b/Source/WebKit/ChangeLog
@@ -1,3 +1,16 @@
+2018-06-21 Chris Dumez <cdumez@apple.com>
+
+ Unreviewed, rolling out r232995.
+
+ Seems to have caused flakiness
+
+ Reverted changeset:
+
+ "Implement IPC throttling to keep the main thread responsive
+ when a process misbehaves"
+ https://bugs.webkit.org/show_bug.cgi?id=186607
+ https://trac.webkit.org/changeset/232995
+
2018-06-15 Jer Noble <jer.noble@apple.com>
Address fullscreen api CSS env feedback
diff --git a/Source/WebKit/Platform/IPC/Connection.cpp b/Source/WebKit/Platform/IPC/Connection.cpp
index d00d8a9..9e27540 100644
--- a/Source/WebKit/Platform/IPC/Connection.cpp
+++ b/Source/WebKit/Platform/IPC/Connection.cpp
@@ -44,11 +44,6 @@
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;
@@ -244,7 +239,6 @@
, m_inDispatchMessageCount(0)
, m_inDispatchMessageMarkedDispatchWhenWaitingForSyncReplyCount(0)
, m_didReceiveInvalidMessage(false)
- , m_incomingMessagesThrottler(*this, &Connection::dispatchIncomingMessages)
, m_waitingForMessage(nullptr)
, m_shouldWaitForSyncReplies(true)
{
@@ -899,28 +893,11 @@
{
{
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->dispatchIncomingMessages();
+ protectedThis->dispatchOneMessage();
});
}
@@ -972,87 +949,19 @@
m_didReceiveInvalidMessage = oldDidReceiveInvalidMessage;
}
-Connection::MessagesThrottler::MessagesThrottler(Connection& connection, DispatchMessagesFunction dispatchMessages)
- : m_dispatchMessagesTimer(RunLoop::main(), &connection, dispatchMessages)
- , m_connection(connection)
- , m_dispatchMessages(dispatchMessages)
+void Connection::dispatchOneMessage()
{
- 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())
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());
- }
-
- // 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()
diff --git a/Source/WebKit/Platform/IPC/Connection.h b/Source/WebKit/Platform/IPC/Connection.h
index 6772aa4..f2598f6 100644
--- a/Source/WebKit/Platform/IPC/Connection.h
+++ b/Source/WebKit/Platform/IPC/Connection.h
@@ -40,7 +40,6 @@
#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>
@@ -232,7 +231,7 @@
void connectionDidClose();
// Called on the listener thread.
- void dispatchIncomingMessages();
+ void dispatchOneMessage();
void dispatchMessage(std::unique_ptr<Decoder>);
void dispatchMessage(Decoder&);
void dispatchSyncMessage(Decoder&);
@@ -241,7 +240,6 @@
// Can be called on any thread.
void enqueueIncomingMessage(std::unique_ptr<Decoder>);
- size_t incomingMessagesDispatchingBatchSize() const;
void willSendSyncMessage(OptionSet<SendSyncOption>);
void didReceiveSyncReply(OptionSet<SendSyncOption>);
@@ -252,21 +250,6 @@
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 };
@@ -292,7 +275,6 @@
// Incoming messages.
Lock m_incomingMessagesMutex;
Deque<std::unique_ptr<Decoder>> m_incomingMessages;
- MessagesThrottler m_incomingMessagesThrottler;
// Outgoing messages.
Lock m_outgoingMessagesMutex;
@@ -357,7 +339,6 @@
bool m_isInitializingSendSource { false };
OSObjectPtr<xpc_connection_t> m_xpcConnection;
- bool m_wasKilled { false };
#elif OS(WINDOWS)
// Called on the connection queue.
void readEventHandler();
diff --git a/Source/WebKit/Platform/IPC/mac/ConnectionMac.mm b/Source/WebKit/Platform/IPC/mac/ConnectionMac.mm
index e154a52..9c7afce 100644
--- a/Source/WebKit/Platform/IPC/mac/ConnectionMac.mm
+++ b/Source/WebKit/Platform/IPC/mac/ConnectionMac.mm
@@ -623,7 +623,6 @@
{
if (m_xpcConnection) {
xpc_connection_kill(m_xpcConnection.get(), SIGKILL);
- m_wasKilled = true;
return true;
}
