Title: [230314] trunk/Source/WebKit
- Revision
- 230314
- Author
- [email protected]
- Date
- 2018-04-05 14:21:50 -0700 (Thu, 05 Apr 2018)
Log Message
WebContent process sometimes hangs in WebProcess::ensureNetworkProcessConnection
https://bugs.webkit.org/show_bug.cgi?id=184326
Reviewed by Chris Dumez.
The hang was caused by UI process never sending the reply back to GetNetworkProcessConnection
due to m_pendingOutgoingMachMessage being set and the event handler for DISPATCH_MACH_SEND_POSSIBLE
never getting called. This is because the event handler registration happens asynchronously,
and may not have completed by the time we send the first IPC to the web content process
in which case it can timeout and we may never get the callback.
Fixed the hang by waiting for the event handler registration to be completed using
dispatch_source_set_registration_handler. To do this, this patch adds a new boolean instance variable,
m_isInitializingSendSource, to Connection which is set to true between the time mach port is created
and until the event handler registration has been completed. platformCanSendOutgoingMessages returns
false while m_isInitializingSendSource is set to prevent the attempt to send messages like we do when
m_pendingOutgoingMachMessage is set to true.
* Platform/IPC/Connection.h:
(IPC::Connection::m_isInitializingSendSource): Added.
* Platform/IPC/mac/ConnectionMac.mm:
(IPC::Connection::platformInvalidate): Set m_isInitializingSendSource to false.
(IPC::Connection::sendMessage): Assert that m_isInitializingSendSource is false.
(IPC::Connection::platformCanSendOutgoingMessages const): Return false if m_isInitializingSendSource
is set to true.
(IPC::Connection::sendOutgoingMessage): Assert that m_isInitializingSendSource is false.
(IPC::Connection::initializeSendSource): Set m_isInitializingSendSource to true temporarily until
dispatch_source_set_registration_handler's callback is called. Resume and send any pending outgoing
messages.
(IPC::Connection::resumeSendSource): Extracted from initializeSendSource.
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (230313 => 230314)
--- trunk/Source/WebKit/ChangeLog 2018-04-05 21:14:31 UTC (rev 230313)
+++ trunk/Source/WebKit/ChangeLog 2018-04-05 21:21:50 UTC (rev 230314)
@@ -1,3 +1,36 @@
+2018-04-05 Ryosuke Niwa <[email protected]>
+
+ WebContent process sometimes hangs in WebProcess::ensureNetworkProcessConnection
+ https://bugs.webkit.org/show_bug.cgi?id=184326
+
+ Reviewed by Chris Dumez.
+
+ The hang was caused by UI process never sending the reply back to GetNetworkProcessConnection
+ due to m_pendingOutgoingMachMessage being set and the event handler for DISPATCH_MACH_SEND_POSSIBLE
+ never getting called. This is because the event handler registration happens asynchronously,
+ and may not have completed by the time we send the first IPC to the web content process
+ in which case it can timeout and we may never get the callback.
+
+ Fixed the hang by waiting for the event handler registration to be completed using
+ dispatch_source_set_registration_handler. To do this, this patch adds a new boolean instance variable,
+ m_isInitializingSendSource, to Connection which is set to true between the time mach port is created
+ and until the event handler registration has been completed. platformCanSendOutgoingMessages returns
+ false while m_isInitializingSendSource is set to prevent the attempt to send messages like we do when
+ m_pendingOutgoingMachMessage is set to true.
+
+ * Platform/IPC/Connection.h:
+ (IPC::Connection::m_isInitializingSendSource): Added.
+ * Platform/IPC/mac/ConnectionMac.mm:
+ (IPC::Connection::platformInvalidate): Set m_isInitializingSendSource to false.
+ (IPC::Connection::sendMessage): Assert that m_isInitializingSendSource is false.
+ (IPC::Connection::platformCanSendOutgoingMessages const): Return false if m_isInitializingSendSource
+ is set to true.
+ (IPC::Connection::sendOutgoingMessage): Assert that m_isInitializingSendSource is false.
+ (IPC::Connection::initializeSendSource): Set m_isInitializingSendSource to true temporarily until
+ dispatch_source_set_registration_handler's callback is called. Resume and send any pending outgoing
+ messages.
+ (IPC::Connection::resumeSendSource): Extracted from initializeSendSource.
+
2018-04-05 Youenn Fablet <[email protected]>
WebRTC data channel only applications require capture permissions for direct connections
Modified: trunk/Source/WebKit/Platform/IPC/Connection.h (230313 => 230314)
--- trunk/Source/WebKit/Platform/IPC/Connection.h 2018-04-05 21:14:31 UTC (rev 230313)
+++ trunk/Source/WebKit/Platform/IPC/Connection.h 2018-04-05 21:21:50 UTC (rev 230314)
@@ -326,6 +326,7 @@
// Called on the connection queue.
void receiveSourceEventHandler();
void initializeSendSource();
+ void resumeSendSource();
mach_port_t m_sendPort { MACH_PORT_NULL };
dispatch_source_t m_sendSource { nullptr };
@@ -334,6 +335,7 @@
dispatch_source_t m_receiveSource { nullptr };
std::unique_ptr<MachMessage> m_pendingOutgoingMachMessage;
+ bool m_isInitializingSendSource { false };
OSObjectPtr<xpc_connection_t> m_xpcConnection;
#elif OS(WINDOWS)
Modified: trunk/Source/WebKit/Platform/IPC/mac/ConnectionMac.mm (230313 => 230314)
--- trunk/Source/WebKit/Platform/IPC/mac/ConnectionMac.mm 2018-04-05 21:14:31 UTC (rev 230313)
+++ trunk/Source/WebKit/Platform/IPC/mac/ConnectionMac.mm 2018-04-05 21:21:50 UTC (rev 230314)
@@ -132,6 +132,7 @@
}
m_pendingOutgoingMachMessage = nullptr;
+ m_isInitializingSendSource = false;
m_isConnected = false;
ASSERT(m_sendPort);
@@ -247,6 +248,7 @@
{
ASSERT(message);
ASSERT(!m_pendingOutgoingMachMessage);
+ ASSERT(!m_isInitializingSendSource);
// Send the message.
kern_return_t kr = mach_msg(message->header(), MACH_SEND_MSG | MACH_SEND_TIMEOUT | MACH_SEND_NOTIFY, message->size(), 0, MACH_PORT_NULL, MACH_MSG_TIMEOUT_NONE, MACH_PORT_NULL);
@@ -273,12 +275,12 @@
bool Connection::platformCanSendOutgoingMessages() const
{
- return !m_pendingOutgoingMachMessage;
+ return !m_pendingOutgoingMachMessage && !m_isInitializingSendSource;
}
bool Connection::sendOutgoingMessage(std::unique_ptr<Encoder> encoder)
{
- ASSERT(!m_pendingOutgoingMachMessage);
+ ASSERT(!m_pendingOutgoingMachMessage && !m_isInitializingSendSource);
Vector<Attachment> attachments = encoder->releaseAttachments();
@@ -371,8 +373,15 @@
void Connection::initializeSendSource()
{
m_sendSource = dispatch_source_create(DISPATCH_SOURCE_TYPE_MACH_SEND, m_sendPort, DISPATCH_MACH_SEND_DEAD | DISPATCH_MACH_SEND_POSSIBLE, m_connectionQueue->dispatchQueue());
+ m_isInitializingSendSource = true;
RefPtr<Connection> connection(this);
+ dispatch_source_set_registration_handler(m_sendSource, [connection] {
+ if (!connection->m_sendSource)
+ return;
+ connection->m_isInitializingSendSource = false;
+ connection->resumeSendSource();
+ });
dispatch_source_set_event_handler(m_sendSource, [connection] {
if (!connection->m_sendSource)
return;
@@ -386,9 +395,7 @@
if (data & DISPATCH_MACH_SEND_POSSIBLE) {
// FIXME: Figure out why we get spurious DISPATCH_MACH_SEND_POSSIBLE events.
- if (connection->m_pendingOutgoingMachMessage)
- connection->sendMessage(WTFMove(connection->m_pendingOutgoingMachMessage));
- connection->sendOutgoingMessages();
+ connection->resumeSendSource();
return;
}
});
@@ -401,6 +408,14 @@
});
}
+void Connection::resumeSendSource()
+{
+ ASSERT(!m_isInitializingSendSource);
+ if (m_pendingOutgoingMachMessage)
+ sendMessage(WTFMove(m_pendingOutgoingMachMessage));
+ sendOutgoingMessages();
+}
+
static std::unique_ptr<Decoder> createMessageDecoder(mach_msg_header_t* header)
{
if (!(header->msgh_bits & MACH_MSGH_BITS_COMPLEX)) {
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
