Bug 58239 - REGRESSION (WebKit2): Deadlock clicking Flash plugin
Summary: REGRESSION (WebKit2): Deadlock clicking Flash plugin
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Plug-ins (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Windows 7
: P2 Normal
Assignee: Nobody
URL: http://frostyschristmastreefarm.com/
Keywords: InRadar, PlatformOnly, Regression
: 53209 53211 (view as bug list)
Depends on: 53211
Blocks:
  Show dependency treegraph
 
Reported: 2011-04-11 09:04 PDT by Adam Roben (:aroben)
Modified: 2011-04-11 14:27 PDT (History)
3 users (show)

See Also:


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 Details | Formatted Diff | Diff
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+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Roben (:aroben) 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.
Comment 1 Adam Roben (:aroben) 2011-04-11 09:04:23 PDT
<rdar://problem/8769302>
Comment 2 Adam Roben (:aroben) 2011-04-11 09:05:03 PDT
See bug 51351 comment 0 for a description of how the deadlock occurs.
Comment 3 Adam Roben (:aroben) 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).
Comment 4 Adam Roben (:aroben) 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.
Comment 5 Adam Roben (:aroben) 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.
Comment 6 Adam Roben (:aroben) 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.)
Comment 7 Adam Roben (:aroben) 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
Comment 8 WebKit Review Bot 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.
Comment 9 Adam Roben (:aroben) 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!
Comment 10 Adam Roben (:aroben) 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
Comment 11 WebKit Review Bot 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.
Comment 12 Anders Carlsson 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.
Comment 13 Adam Roben (:aroben) 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.
Comment 14 Adam Roben (:aroben) 2011-04-11 14:26:49 PDT
Committed r83498: <http://trac.webkit.org/changeset/83498>
Comment 15 Adam Roben (:aroben) 2011-04-11 14:27:25 PDT
*** Bug 53211 has been marked as a duplicate of this bug. ***
Comment 16 Adam Roben (:aroben) 2011-04-11 14:27:57 PDT
*** Bug 53209 has been marked as a duplicate of this bug. ***