Bug 221488

Summary: WebGL IPC messages are delivered out of order
Product: WebKit Reporter: Kimmo Kinnunen <kkinnunen>
Component: WebGLAssignee: Kimmo Kinnunen <kkinnunen>
Status: RESOLVED FIXED    
Severity: Normal CC: ap, cdumez, dino, kbr, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Local Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=221566
Bug Depends on: 221396    
Bug Blocks: 217211, 219641    
Attachments:
Description Flags
Patch
none
new approach none

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
DetailsFormatted DiffDiff
new approach (3.85 KB, patch)
2021-02-10 12:13 PST, Kimmo Kinnunen 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Kimmo Kinnunen
Comment 1 2021-02-05 10:37:34 PST
Created attachment 419430 [details] Patch
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
<rdar://problem/74206986>
Note You need to log in before you can comment on or make changes to this bug.