- Revision
- 280736
- Author
- akeer...@apple.com
- Date
- 2021-08-06 14:59:14 -0700 (Fri, 06 Aug 2021)
Log Message
[macOS] 3 second IPC deadlocks involving WebPageProxy::acceptsFirstMouse
https://bugs.webkit.org/show_bug.cgi?id=228834
rdar://75390908
Reviewed by Wenson Hsieh and Tim Horton.
Reports show 3 second hangs in WebPageProxy::acceptsFirstMouse, which is
the timeout for the sync IPC sent by the method. While this method has
always sent a sync IPC, logs show the WebProcess is also blocked on sync
IPC, under WebProcess::ensureGPUProcessConnection.
If a Web<->GPU process connection has not been established,
WebProcess::ensureGPUProcessConnection sends a sync IPC from the
WebProcess to the UIProcess to establish get the connection. However, to
get the connection, the UIProcess then sends an async IPC with a reply
block to the GPUProcess (see GPUProcessProxy::getGPUProcessConnection).
The WebProcess remains blocked until the UIProcess receives the reply
from the GPUProcess, and then sends a reply to the WebProcess.
Now, if a call to acceptsFirstMouse occurs in between the time the async
UIProcess -> GPUProcess IPC is sent, and the reply is received, we will
experience deadlock. The UIProcess will be blocked waiting on the
WebProcess' reply to acceptsFirstMouse, and the reply from the GPUProcess
will not be dispatched. Since the reply from the GPUProcess will not be
dispatched until the UIProcess is unblocked, the WebProcess will be
waiting under WebProcess:ensureGPUProcessConnection, and unable to reply
to acceptsFirstMouse.
A simple fix for the described scenario would be to introduce a
replySendOptions parameter to sendWithAsyncReply, so that the UIProcess
could be re-entered to dispatch the GPUProcess reply, even when waiting
for acceptsFirstMouse. However, this will not work in cases where the
GPUProcess is being launched, due to being blocked on the reply from
xpc_connection_send_message_with_reply in ProcessLauncherMac, which is
before the IPC::Connection has been established.
Since re-entering the UIProcess is not possible in this scenario, given
the existing architecture, the next simplest solution is to re-enter
the WebProcess on acceptsFirstMouse instead. Consequently, one part of
the fix involves sending the acceptsFirstMouse IPC with
IPC::SendOption::DispatchMessageEvenWhenWaitingForUnboundedSyncReply and
tagging WebProcess::getGPUProcessConnection as unbounded sync IPC.
Then, we must also consider the fact that, in the WebProcess,
acceptsFirstMouse performs hit-testing. Hit-testing triggers layout and
could cause script to run. Script could result in a GPUProcess connection
being established, eventually leading us down the path towards the
aforementioned deadlock. To avoid deadlock in this scenario, we need to
interrupt acceptsFirstMouse if a message to get the GPUProcess connection
is received. This is achieved by using waitForAndDispatchImmediately with
the InterruptWaitingIfSyncMessageArrives option, and splitting up
acceptsFirstMouse into two separate IPCs.
* UIProcess/WebPageProxy.h:
* UIProcess/WebPageProxy.messages.in:
* UIProcess/ios/WebPageProxyIOS.mm:
Implementation not required for iOS.
* UIProcess/mac/WebPageProxyMac.mm:
(WebKit::WebPageProxy::acceptsFirstMouse):
Send an async IPC with IPC::SendOption::DispatchMessageEvenWhenWaitingForUnboundedSyncReply
so that the WebProcess can be re-entered to resolve deadlock if
necessary. Then, use waitForAndDispatchImmediately to wait for the reply
(essentially making it a sync IPC), so that the wait can be interrupted
if another sync message, such as getGPUProcessConnection, is received.
(WebKit::WebPageProxy::handleAcceptsFirstMouse):
Store the result of the reply from the WebProcess.
* WebProcess/WebPage/WebPage.h:
* WebProcess/WebPage/WebPage.messages.in:
Split the sync IPC into two async IPCs, in order to support the use of
waitForAndDispatchImmediately.
* WebProcess/WebPage/ios/WebPageIOS.mm:
Implementation not required for iOS.
* WebProcess/WebPage/mac/WebPageMac.mm:
(WebKit::WebPage::requestAcceptsFirstMouse):
Exit early if re-entering the WebProcess, to avoid running script on
re-entry.
* WebProcess/WebProcess.cpp:
(WebKit::WebProcess::getGPUProcessConnection):
Tag as unbounded sync IPC, so the WebProcess can be re-entered to
resolve deadlock.
Modified Paths
Diff
Modified: trunk/Source/WebKit/ChangeLog (280735 => 280736)
--- trunk/Source/WebKit/ChangeLog 2021-08-06 21:45:01 UTC (rev 280735)
+++ trunk/Source/WebKit/ChangeLog 2021-08-06 21:59:14 UTC (rev 280736)
@@ -1,3 +1,99 @@
+2021-08-06 Aditya Keerthi <akeer...@apple.com>
+
+ [macOS] 3 second IPC deadlocks involving WebPageProxy::acceptsFirstMouse
+ https://bugs.webkit.org/show_bug.cgi?id=228834
+ rdar://75390908
+
+ Reviewed by Wenson Hsieh and Tim Horton.
+
+ Reports show 3 second hangs in WebPageProxy::acceptsFirstMouse, which is
+ the timeout for the sync IPC sent by the method. While this method has
+ always sent a sync IPC, logs show the WebProcess is also blocked on sync
+ IPC, under WebProcess::ensureGPUProcessConnection.
+
+ If a Web<->GPU process connection has not been established,
+ WebProcess::ensureGPUProcessConnection sends a sync IPC from the
+ WebProcess to the UIProcess to establish get the connection. However, to
+ get the connection, the UIProcess then sends an async IPC with a reply
+ block to the GPUProcess (see GPUProcessProxy::getGPUProcessConnection).
+ The WebProcess remains blocked until the UIProcess receives the reply
+ from the GPUProcess, and then sends a reply to the WebProcess.
+
+ Now, if a call to acceptsFirstMouse occurs in between the time the async
+ UIProcess -> GPUProcess IPC is sent, and the reply is received, we will
+ experience deadlock. The UIProcess will be blocked waiting on the
+ WebProcess' reply to acceptsFirstMouse, and the reply from the GPUProcess
+ will not be dispatched. Since the reply from the GPUProcess will not be
+ dispatched until the UIProcess is unblocked, the WebProcess will be
+ waiting under WebProcess:ensureGPUProcessConnection, and unable to reply
+ to acceptsFirstMouse.
+
+ A simple fix for the described scenario would be to introduce a
+ replySendOptions parameter to sendWithAsyncReply, so that the UIProcess
+ could be re-entered to dispatch the GPUProcess reply, even when waiting
+ for acceptsFirstMouse. However, this will not work in cases where the
+ GPUProcess is being launched, due to being blocked on the reply from
+ xpc_connection_send_message_with_reply in ProcessLauncherMac, which is
+ before the IPC::Connection has been established.
+
+ Since re-entering the UIProcess is not possible in this scenario, given
+ the existing architecture, the next simplest solution is to re-enter
+ the WebProcess on acceptsFirstMouse instead. Consequently, one part of
+ the fix involves sending the acceptsFirstMouse IPC with
+ IPC::SendOption::DispatchMessageEvenWhenWaitingForUnboundedSyncReply and
+ tagging WebProcess::getGPUProcessConnection as unbounded sync IPC.
+
+ Then, we must also consider the fact that, in the WebProcess,
+ acceptsFirstMouse performs hit-testing. Hit-testing triggers layout and
+ could cause script to run. Script could result in a GPUProcess connection
+ being established, eventually leading us down the path towards the
+ aforementioned deadlock. To avoid deadlock in this scenario, we need to
+ interrupt acceptsFirstMouse if a message to get the GPUProcess connection
+ is received. This is achieved by using waitForAndDispatchImmediately with
+ the InterruptWaitingIfSyncMessageArrives option, and splitting up
+ acceptsFirstMouse into two separate IPCs.
+
+ * UIProcess/WebPageProxy.h:
+ * UIProcess/WebPageProxy.messages.in:
+ * UIProcess/ios/WebPageProxyIOS.mm:
+
+ Implementation not required for iOS.
+
+ * UIProcess/mac/WebPageProxyMac.mm:
+ (WebKit::WebPageProxy::acceptsFirstMouse):
+
+ Send an async IPC with IPC::SendOption::DispatchMessageEvenWhenWaitingForUnboundedSyncReply
+ so that the WebProcess can be re-entered to resolve deadlock if
+ necessary. Then, use waitForAndDispatchImmediately to wait for the reply
+ (essentially making it a sync IPC), so that the wait can be interrupted
+ if another sync message, such as getGPUProcessConnection, is received.
+
+ (WebKit::WebPageProxy::handleAcceptsFirstMouse):
+
+ Store the result of the reply from the WebProcess.
+
+ * WebProcess/WebPage/WebPage.h:
+ * WebProcess/WebPage/WebPage.messages.in:
+
+ Split the sync IPC into two async IPCs, in order to support the use of
+ waitForAndDispatchImmediately.
+
+ * WebProcess/WebPage/ios/WebPageIOS.mm:
+
+ Implementation not required for iOS.
+
+ * WebProcess/WebPage/mac/WebPageMac.mm:
+ (WebKit::WebPage::requestAcceptsFirstMouse):
+
+ Exit early if re-entering the WebProcess, to avoid running script on
+ re-entry.
+
+ * WebProcess/WebProcess.cpp:
+ (WebKit::WebProcess::getGPUProcessConnection):
+
+ Tag as unbounded sync IPC, so the WebProcess can be re-entered to
+ resolve deadlock.
+
2021-08-06 Youenn Fablet <you...@apple.com>
WebKit::SampleBufferDisplayLayer needs to handle GPUProcess crash
Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.h (280735 => 280736)
--- trunk/Source/WebKit/UIProcess/WebPageProxy.h 2021-08-06 21:45:01 UTC (rev 280735)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.h 2021-08-06 21:59:14 UTC (rev 280736)
@@ -913,7 +913,6 @@
void sendComplexTextInputToPlugin(uint64_t pluginComplexTextInputIdentifier, const String& textInput);
bool shouldDelayWindowOrderingForEvent(const WebMouseEvent&);
- bool acceptsFirstMouse(int eventNumber, const WebMouseEvent&);
void setRemoteLayerTreeRootNode(RemoteLayerTreeNode*);
CALayer *acceleratedCompositingRootLayer() const;
@@ -1921,6 +1920,8 @@
#if PLATFORM(MAC)
void changeUniversalAccessZoomFocus(const WebCore::IntRect&, const WebCore::IntRect&);
+
+ bool acceptsFirstMouse(int eventNumber, const WebMouseEvent&);
#endif
void dispatchWheelEventWithoutScrolling(const WebWheelEvent&, CompletionHandler<void(bool)>&&);
@@ -2324,6 +2325,8 @@
void recordAutocorrectionResponse(int32_t responseType, const String& replacedString, const String& replacementString);
void setEditableElementIsFocused(bool);
+
+ void handleAcceptsFirstMouse(bool);
#endif // PLATFORM(MAC)
#if PLATFORM(IOS_FAMILY)
@@ -2613,6 +2616,8 @@
#if PLATFORM(MAC)
bool m_useSystemAppearance { false };
+
+ bool m_acceptsFirstMouse { false };
#endif
#if ENABLE(APPLE_PAY)
Modified: trunk/Source/WebKit/UIProcess/WebPageProxy.messages.in (280735 => 280736)
--- trunk/Source/WebKit/UIProcess/WebPageProxy.messages.in 2021-08-06 21:45:01 UTC (rev 280735)
+++ trunk/Source/WebKit/UIProcess/WebPageProxy.messages.in 2021-08-06 21:59:14 UTC (rev 280736)
@@ -376,6 +376,8 @@
RecordAutocorrectionResponse(int32_t response, String replacedString, String replacementString);
SetEditableElementIsFocused(bool editableElementIsFocused)
+
+ HandleAcceptsFirstMouse(bool acceptsFirstMouse)
#endif
DidUpdateRenderingAfterCommittingLoad()
Modified: trunk/Source/WebKit/UIProcess/ios/WebPageProxyIOS.mm (280735 => 280736)
--- trunk/Source/WebKit/UIProcess/ios/WebPageProxyIOS.mm 2021-08-06 21:45:01 UTC (rev 280735)
+++ trunk/Source/WebKit/UIProcess/ios/WebPageProxyIOS.mm 2021-08-06 21:59:14 UTC (rev 280736)
@@ -774,12 +774,6 @@
return false;
}
-bool WebPageProxy::acceptsFirstMouse(int, const WebKit::WebMouseEvent&)
-{
- notImplemented();
- return false;
-}
-
void WebPageProxy::willStartUserTriggeredZooming()
{
send(Messages::WebPage::WillStartUserTriggeredZooming());
Modified: trunk/Source/WebKit/UIProcess/mac/WebPageProxyMac.mm (280735 => 280736)
--- trunk/Source/WebKit/UIProcess/mac/WebPageProxyMac.mm 2021-08-06 21:45:01 UTC (rev 280735)
+++ trunk/Source/WebKit/UIProcess/mac/WebPageProxyMac.mm 2021-08-06 21:59:14 UTC (rev 280736)
@@ -49,6 +49,7 @@
#import "WKSharingServicePickerDelegate.h"
#import "WebContextMenuProxyMac.h"
#import "WebPageMessages.h"
+#import "WebPageProxyMessages.h"
#import "WebPreferencesKeys.h"
#import "WebProcessProxy.h"
#import <WebCore/AttributedString.h>
@@ -395,12 +396,23 @@
if (!hasRunningProcess())
return false;
- bool result = false;
- const Seconds messageTimeout(3);
- sendSync(Messages::WebPage::AcceptsFirstMouse(eventNumber, event), Messages::WebPage::AcceptsFirstMouse::Reply(result), messageTimeout);
- return result;
+ if (!m_process->hasConnection())
+ return false;
+
+ send(Messages::WebPage::RequestAcceptsFirstMouse(eventNumber, event), IPC::SendOption::DispatchMessageEvenWhenWaitingForUnboundedSyncReply);
+ bool receivedReply = m_process->connection()->waitForAndDispatchImmediately<Messages::WebPageProxy::HandleAcceptsFirstMouse>(webPageID(), 3_s, IPC::WaitForOption::InterruptWaitingIfSyncMessageArrives);
+
+ if (!receivedReply)
+ return false;
+
+ return m_acceptsFirstMouse;
}
+void WebPageProxy::handleAcceptsFirstMouse(bool acceptsFirstMouse)
+{
+ m_acceptsFirstMouse = acceptsFirstMouse;
+}
+
void WebPageProxy::setRemoteLayerTreeRootNode(RemoteLayerTreeNode* rootNode)
{
pageClient().setRemoteLayerTreeRootNode(rootNode);
Modified: trunk/Source/WebKit/WebProcess/WebPage/WebPage.h (280735 => 280736)
--- trunk/Source/WebKit/WebProcess/WebPage/WebPage.h 2021-08-06 21:45:01 UTC (rev 280735)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebPage.h 2021-08-06 21:59:14 UTC (rev 280736)
@@ -930,7 +930,6 @@
void getStringSelectionForPasteboard(CompletionHandler<void(String&&)>&&);
void getDataSelectionForPasteboard(const String pasteboardType, CompletionHandler<void(SharedMemory::IPCHandle&&)>&&);
void shouldDelayWindowOrderingEvent(const WebKit::WebMouseEvent&, CompletionHandler<void(bool)>&&);
- void acceptsFirstMouse(int eventNumber, const WebKit::WebMouseEvent&, CompletionHandler<void(bool)>&&);
bool performNonEditingBehaviorForSelector(const String&, WebCore::KeyboardEvent*);
void insertDictatedTextAsync(const String& text, const EditingRange& replacementRange, const Vector<WebCore::DictationAlternative>& dictationAlternativeLocations, InsertTextOptions&&);
@@ -939,6 +938,7 @@
#if PLATFORM(MAC)
void attributedSubstringForCharacterRangeAsync(const EditingRange&, CompletionHandler<void(const WebCore::AttributedString&, const EditingRange&)>&&);
void fontAtSelection(CompletionHandler<void(const FontInfo&, double, bool)>&&);
+ void requestAcceptsFirstMouse(int eventNumber, const WebKit::WebMouseEvent&);
#endif
#if PLATFORM(COCOA) && ENABLE(SERVICE_CONTROLS)
Modified: trunk/Source/WebKit/WebProcess/WebPage/WebPage.messages.in (280735 => 280736)
--- trunk/Source/WebKit/WebProcess/WebPage/WebPage.messages.in 2021-08-06 21:45:01 UTC (rev 280735)
+++ trunk/Source/WebKit/WebProcess/WebPage/WebPage.messages.in 2021-08-06 21:59:14 UTC (rev 280736)
@@ -446,7 +446,6 @@
#endif
ShouldDelayWindowOrderingEvent(WebKit::WebMouseEvent event) -> (bool result) Synchronous
- AcceptsFirstMouse(int eventNumber, WebKit::WebMouseEvent event) -> (bool result) Synchronous
SetTextAsync(String text)
@@ -464,6 +463,7 @@
#if PLATFORM(MAC)
AttributedSubstringForCharacterRangeAsync(struct WebKit::EditingRange range) -> (struct WebCore::AttributedString string, struct WebKit::EditingRange range) Async
FontAtSelection() -> (struct WebKit::FontInfo fontInfo, double fontSize, bool selectionHasMultipleFonts) Async
+ RequestAcceptsFirstMouse(int eventNumber, WebKit::WebMouseEvent event)
#endif
SetAlwaysShowsHorizontalScroller(bool alwaysShowsHorizontalScroller)
Modified: trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm (280735 => 280736)
--- trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm 2021-08-06 21:45:01 UTC (rev 280735)
+++ trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm 2021-08-06 21:59:14 UTC (rev 280736)
@@ -636,12 +636,6 @@
completionHandler(false);
}
-void WebPage::acceptsFirstMouse(int, const WebKit::WebMouseEvent&, CompletionHandler<void(bool)>&& completionHandler)
-{
- notImplemented();
- completionHandler(false);
-}
-
void WebPage::computePagesForPrintingPDFDocument(WebCore::FrameIdentifier, const PrintInfo&, Vector<IntRect>&)
{
notImplemented();
Modified: trunk/Source/WebKit/WebProcess/WebPage/mac/WebPageMac.mm (280735 => 280736)
--- trunk/Source/WebKit/WebProcess/WebPage/mac/WebPageMac.mm 2021-08-06 21:45:01 UTC (rev 280735)
+++ trunk/Source/WebKit/WebProcess/WebPage/mac/WebPageMac.mm 2021-08-06 21:59:14 UTC (rev 280736)
@@ -603,12 +603,13 @@
completionHandler(result);
}
-void WebPage::acceptsFirstMouse(int eventNumber, const WebKit::WebMouseEvent& event, CompletionHandler<void(bool)>&& completionHandler)
+void WebPage::requestAcceptsFirstMouse(int eventNumber, const WebKit::WebMouseEvent& event)
{
if (WebProcess::singleton().parentProcessConnection()->inSendSync()) {
// In case we're already inside a sendSync message, it's possible that the page is in a
// transitionary state, so any hit-testing could cause crashes so we just return early in that case.
- return completionHandler(false);
+ send(Messages::WebPageProxy::HandleAcceptsFirstMouse(false));
+ return;
}
auto& frame = m_page->focusController().focusedOrMainFrame();
@@ -623,7 +624,8 @@
else
#endif
result = !!hitResult.scrollbar();
- completionHandler(result);
+
+ send(Messages::WebPageProxy::HandleAcceptsFirstMouse(result));
}
void WebPage::setTopOverhangImage(WebImage* image)
Modified: trunk/Source/WebKit/WebProcess/WebProcess.cpp (280735 => 280736)
--- trunk/Source/WebKit/WebProcess/WebProcess.cpp 2021-08-06 21:45:01 UTC (rev 280735)
+++ trunk/Source/WebKit/WebProcess/WebProcess.cpp 2021-08-06 21:59:14 UTC (rev 280736)
@@ -1213,6 +1213,8 @@
GPUProcessConnectionParameters parameters;
platformInitializeGPUProcessConnectionParameters(parameters);
+ IPC::UnboundedSynchronousIPCScope unboundedSynchronousIPCScope;
+
GPUProcessConnectionInfo connectionInfo;
if (!connection.sendSync(Messages::WebProcessProxy::GetGPUProcessConnection(parameters), Messages::WebProcessProxy::GetGPUProcessConnection::Reply(connectionInfo), 0)) {
// If we failed the first time, retry once. The attachment may have become invalid
