RESOLVED FIXED 221488
WebGL IPC messages are delivered out of order
https://bugs.webkit.org/show_bug.cgi?id=221488
Summary WebGL IPC messages are delivered out of order
Kimmo Kinnunen
Reported 2021-02-05 10:30:47 PST
WebGL IPC messages are delivered out of order Sync messages are delivered before earlier async messages, if some other WebKit part waits on sync replies.
Attachments
Patch (4.65 KB, patch)
2021-02-05 10:37 PST, Kimmo Kinnunen
no flags
new approach (3.85 KB, patch)
2021-02-10 12:13 PST, Kimmo Kinnunen
no flags
Kimmo Kinnunen
Comment 1 2021-02-05 10:37:34 PST
Kenneth Russell
Comment 2 2021-02-05 16:02:15 PST
Comment on attachment 419430 [details] Patch Looks good to me. (The EWS failures are all failures to apply the patch.) Chromium had this exact bug in its IPC subsystem some time ago and I recall working on the fix. r+
Alexey Proskuryakov
Comment 3 2021-02-08 11:03:37 PST
I'm somewhat skeptical that this won't introduce deadlocks in more cases than you are warning about in the comment. I won't have an opportunity to think it through, so feel free to ignore my comment, but it may be good to give Chris some time to comment.
Chris Dumez
Comment 4 2021-02-08 11:11:10 PST
Comment on attachment 419430 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=419430&action=review > Source/WebKit/Platform/IPC/Connection.h:77 > + // Note: causes timeouts if all parties in the synchronous message chain send synchronous messages with this flag This does not look right to me. We forbid re-entrency from IPC in the WebProcess due to security bugs. Therefore, the other process must always break the deadlock by dispatching when waiting for a sync message.
Chris Dumez
Comment 5 2021-02-08 12:20:08 PST
(In reply to Chris Dumez from comment #4) > Comment on attachment 419430 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=419430&action=review > > > Source/WebKit/Platform/IPC/Connection.h:77 > > + // Note: causes timeouts if all parties in the synchronous message chain send synchronous messages with this flag > > This does not look right to me. We forbid re-entrency from IPC in the > WebProcess due to security bugs. Therefore, the other process must always > break the deadlock by dispatching when waiting for a sync message. This is the logic that prevents WebProcess re-entrency on IPC: // Use this flag to force synchronous messages to be treated as asynchronous messages in the WebProcess. // Otherwise, the WebProcess would process incoming synchronous IPC while waiting for a synchronous IPC // reply from the Network process, which would be unsafe. m_connection->setOnlySendMessagesAsDispatchWhenWaitingForSyncReplyWhenProcessingSuchAMessage(true); We are currently missing this logic on the GPUProcess connection but this is a bug that I will fix.
Kimmo Kinnunen
Comment 6 2021-02-10 12:13:54 PST
Created attachment 419886 [details] new approach
EWS
Comment 7 2021-02-10 13:04:20 PST
Committed r272680: <https://commits.webkit.org/r272680> All reviewed patches have been landed. Closing bug and clearing flags on attachment 419886 [details].
Radar WebKit Bug Importer
Comment 8 2021-02-10 14:40:42 PST
Note You need to log in before you can comment on or make changes to this bug.