RESOLVED FIXED 58239
REGRESSION (WebKit2): Deadlock clicking Flash plugin
https://bugs.webkit.org/show_bug.cgi?id=58239
Summary REGRESSION (WebKit2): Deadlock clicking Flash plugin
Adam Roben (:aroben)
Reported 2011-04-11 09:04:10 PDT
To reproduce: 1. Go to http://frostyschristmastreefarm.com/ 2. Click on the "PB" icon in the Flash audio player near the top of the page The UI process and web process deadlock.
Attachments
Dispatch sent messages to windows owned by the web process when waiting a sync CoreIPC reply (17.78 KB, patch)
2011-04-11 13:52 PDT, Adam Roben (:aroben)
no flags
Dispatch sent messages to windows owned by the web process when waiting a sync CoreIPC reply (16.67 KB, patch)
2011-04-11 13:55 PDT, Adam Roben (:aroben)
andersca: review+
Adam Roben (:aroben)
Comment 1 2011-04-11 09:04:23 PDT
Adam Roben (:aroben)
Comment 2 2011-04-11 09:05:03 PDT
See bug 51351 comment 0 for a description of how the deadlock occurs.
Adam Roben (:aroben)
Comment 3 2011-04-11 09:08:27 PDT
As described in bug 51351 comment 4, we're planning to fix this by making the web process spin a run loop when waiting for a reply to any synchronous message. Bug 51351 and bug 51352 represent future improvements which would allow us to only spin a run loop in certain situations (and thus hopefully reduce chances of bugs due to reentrancy issues).
Adam Roben (:aroben)
Comment 4 2011-04-11 09:45:23 PDT
I'm currently hoping that we can get away with only dispatching messages to web process windows that are descendants of UI process windows. And hopefully we can get away with only dispatching sent (as opposed to posted) messages.
Adam Roben (:aroben)
Comment 5 2011-04-11 09:46:51 PDT
It looks like that is too restrictive. It causes deadlocks when going to cuteoverload.com and clicking on the "YouTube" button in a video. It looks like Flash creates some top-level windows, even for windowless plugins. Maybe we need to process messages for them, too.
Adam Roben (:aroben)
Comment 6 2011-04-11 09:56:18 PDT
Delivering sent messages to top-level windows in the web process seems to fix the cuteoverload.com deadlock. (We are not delivering messages to the RunLoop window, which is good.)
Adam Roben (:aroben)
Comment 7 2011-04-11 13:52:42 PDT
Created attachment 89076 [details] Dispatch sent messages to windows owned by the web process when waiting a sync CoreIPC reply
WebKit Review Bot
Comment 8 2011-04-11 13:54:43 PDT
Attachment 89076 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/page/PageGroup.cpp', u'Sour..." exit_code: 1 Source/WebKit2/Platform/RunLoop.h:47: Code inside a namespace should not be indented. [whitespace/indent] [4] Total errors found: 1 in 13 files If any of these errors are false positives, please file a bug against check-webkit-style.
Adam Roben (:aroben)
Comment 9 2011-04-11 13:55:14 PDT
Comment on attachment 89076 [details] Dispatch sent messages to windows owned by the web process when waiting a sync CoreIPC reply Whoa, the WebCore parts of this patch should not be there!
Adam Roben (:aroben)
Comment 10 2011-04-11 13:55:51 PDT
Created attachment 89079 [details] Dispatch sent messages to windows owned by the web process when waiting a sync CoreIPC reply
WebKit Review Bot
Comment 11 2011-04-11 13:58:57 PDT
Attachment 89079 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit..." exit_code: 1 Source/WebKit2/Platform/RunLoop.h:47: Code inside a namespace should not be indented. [whitespace/indent] [4] Total errors found: 1 in 11 files If any of these errors are false positives, please file a bug against check-webkit-style.
Anders Carlsson
Comment 12 2011-04-11 14:03:51 PDT
Comment on attachment 89079 [details] Dispatch sent messages to windows owned by the web process when waiting a sync CoreIPC reply View in context: https://bugs.webkit.org/attachment.cgi?id=89079&action=review > Source/WebKit2/Platform/CoreIPC/Connection.cpp:56 > + bool waitWhileDispatchingSentMessages(double absoluteTime, const Vector<HWND>& windowsToReceiveMessages) This should be made more clear that it's window messages that are being dispatched.
Adam Roben (:aroben)
Comment 13 2011-04-11 14:16:21 PDT
Comment on attachment 89079 [details] Dispatch sent messages to windows owned by the web process when waiting a sync CoreIPC reply View in context: https://bugs.webkit.org/attachment.cgi?id=89079&action=review >> Source/WebKit2/Platform/CoreIPC/Connection.cpp:56 >> + bool waitWhileDispatchingSentMessages(double absoluteTime, const Vector<HWND>& windowsToReceiveMessages) > > This should be made more clear that it's window messages that are being dispatched. Renamed to waitWhileDispatchingSentWin32Messages.
Adam Roben (:aroben)
Comment 14 2011-04-11 14:26:49 PDT
Adam Roben (:aroben)
Comment 15 2011-04-11 14:27:25 PDT
*** Bug 53211 has been marked as a duplicate of this bug. ***
Adam Roben (:aroben)
Comment 16 2011-04-11 14:27:57 PDT
*** Bug 53209 has been marked as a duplicate of this bug. ***
Note You need to log in before you can comment on or make changes to this bug.