51351 2010-12-20 13:21:43 -0800 UI process should respond to synchronous messages from the web process on a non-main thread by default 2015-06-18 14:56:20 -0700 1 1 1 Unclassified WebKit WebKit2 528+ (Nightly build) All All RESOLVED INVALID InRadar P2 Normal --- 51352 1 aroben webkit-unassigned andersca sam oldest_to_newest 324536 0 aroben 2010-12-20 13:21:43 -0800 The UI process currently responds to both synchronous and asynchronous messages from the web process on the main thread. Messages are received on the CoreIPC::Connection's work queue and then forwarded to the main thread for processing. This can cause deadlocks on Windows when windowed plugins are involved. For example, take the case of a YouTube video embedded into some third party web page. Here's what happens: Web process: Clicking on the plugin opens a new window so that it can be navigated to the video's YouTube page. In order to open the new window, the web process sends a synchronous WebPageProxy::CreateNewPage message and blocks its main thread to wait for the reply. UI process: The UI process receives the WebPageProxy::CreateNewPage message, and ends up calling through to WKPageUIClient::createNewPage. The client app responds to this callback by creating a new window and selecting it. Selecting the new window causes the plugin to lose focus. As part of unfocusing the plugin, Windows sends the plugin a synchronous WM_KILLFOCUS message to it and blocks the main thread while waiting for the message to be handled. Since the web process's main thread is still blocked waiting for the reply to WebPageProxy::CreateNewPage, it can't process the WM_KILLFOCUS message, so the two processes deadlock. (The web process is waiting for CreateNewPage to be handled, and the UI process is waiting for WM_KILLFOCUS to be handled.) We should make the UI process respond to synchronous messages on a non-main thread by default. We'll want to continue making WK2 client callbacks (e.g., WKPageUIClient::createNewPage) on the main thread, but those need to happen asynchronously to avoid deadlocks. 324538 1 aroben 2010-12-20 13:24:13 -0800 (In reply to comment #0) > We should make the UI process respond to synchronous messages on a non-main thread by default. Messages that end up displaying UI still need to be processed on the main thread. Fixing deadlocks with those messages is covered by bug 51352. 324539 2 aroben 2010-12-20 13:24:46 -0800 <rdar://problem/8790535> 340616 3 aroben 2011-01-26 17:19:34 -0800 We've decided to see if we can get away without this. We'll still have to fix bug 51352. 340673 4 aroben 2011-01-26 18:08:23 -0800 (In reply to comment #3) > We've decided to see if we can get away without this. We'll still have to fix bug 51352. Anders and I just talked some more and realized that we will probably need to fix this eventually. Consider the following situation: Web process: Sends some asynchronous messages to the UI process, then sends a synchronous message (that doesn't present UI and thus is not covered by bug 51352, e.g., GetToolbarsAreVisible), and blocks waiting for a reply. UI process: While processing one of the asynchronous messages, touches the window hierarchy, resulting in a synchronous message to the web process. So we've ended up in a deadlock again even if bug 51352 is fixed. Our plan is threefold: 1) Make the web process spin a run loop while waiting for a reply to any synchronous message, not just those that end up presenting UI in the UI process. 2) Add a test mode to WebKit2 that exercises the reentrancy that spinning a nested run loop enables. 3) Eventually change WebKit2 only to spin a run loop while waiting for a reply to a synchronous message that will present UI, and move all other synchronous message handling to a non-main thread, as described in comment 1. If the testing from (2) uncovers many intractable issues, we may do (3) sooner rather than later. 340677 5 aroben 2011-01-26 18:11:14 -0800 Whoops, forgot to reopen the bug. 349280 6 aroben 2011-02-10 12:37:57 -0800 The deadlocks caused by this bug are even more frequent now that decidePolicyForNavigationAction is synchronous. 383198 7 aroben 2011-04-11 09:07:15 -0700 (In reply to comment #4) > 1) Make the web process spin a run loop while waiting for a reply to any synchronous message, not just those that end up presenting UI in the UI process. This is now bug 58239. > 2) Add a test mode to WebKit2 that exercises the reentrancy that spinning a nested run loop enables. This is bug 53218. > 3) Eventually change WebKit2 only to spin a run loop while waiting for a reply to a synchronous message that will present UI, and move all other synchronous message handling to a non-main thread, as described in comment 1. This is bug 51352. 1103044 8 andersca 2015-06-18 14:56:20 -0700 We got rid of WK2 for Windows so this no longer applies.