Bug 58239 – REGRESSION (WebKit2): Deadlock clicking Flash plugin

• Component: Plug-ins
• Status: RESOLVED FIXED

[Adam Roben (:aroben)]

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.

[Adam Roben (:aroben)]

<rdar://problem/8769302>

[Adam Roben (:aroben)]

See bug 51351 comment 0 for a description of how the deadlock occurs.

[Adam Roben (:aroben)]

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)]

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)]

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)]

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)]

Created attachment 89076 [details]
Dispatch sent messages to windows owned by the web process when waiting a sync CoreIPC reply

[WebKit Review Bot]

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 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)]

Created attachment 89079 [details]
Dispatch sent messages to windows owned by the web process when waiting a sync CoreIPC reply

[WebKit Review Bot]

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 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 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)]

Committed r83498: <http://trac.webkit.org/changeset/83498>

[Adam Roben (:aroben)]

*** Bug 53211 has been marked as a duplicate of this bug. ***

[Adam Roben (:aroben)]

*** Bug 53209 has been marked as a duplicate of this bug. ***