Bug 239905

Summary: Connecting to GPU process may hang if UI process sends sync message simultaneously
Product: WebKit Reporter: Kimmo Kinnunen <kkinnunen>
Component: WebKit2Assignee: Kimmo Kinnunen <kkinnunen>
Status: RESOLVED FIXED    
Severity: Normal CC: cdumez, kkinnunen, simon.fraser, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
WIP
none
Patch
none
Patch
none
For landing none

Description Kimmo Kinnunen 2022-04-29 11:03:41 PDT 
Connecting to GPU process may hang if UI process sends sync message simultaneously
Comment 1 Kimmo Kinnunen 2022-04-29 11:07:37 PDT 
Created attachment 458598 [details]
WIP
Comment 2 Kimmo Kinnunen 2022-04-29 11:09:20 PDT 
<rdar://91713027>
Comment 3 Kimmo Kinnunen 2022-05-04 08:35:07 PDT 
Created attachment 458799 [details]
Patch
Comment 4 Kimmo Kinnunen 2022-05-04 08:46:42 PDT 
Created attachment 458801 [details]
Patch
Comment 5 Chris Dumez 2022-05-04 16:17:26 PDT 
Comment on attachment 458801 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=458801&action=review

r=me with fixes. We may want to do the same for the network process?

> Source/WebKit/ChangeLog:23
> +        connection initialization. Send the GPUPg connection initialization result, audit token and

Typo: GPUPg

> Source/WebKit/GPUProcess/GPUProcess.cpp:134
> +    WTFLogAlways("GOT Create gpu process connection");

Should drop.

> Source/WebKit/UIProcess/GPU/GPUProcessProxy.cpp:374
> +    WTFLogAlways("Create gpu process connection");

Should drop

> Source/WebKit/WebProcess/GPU/GPUProcessConnection.cpp:132
> +        if (parentConnection.ignoreInvalidMessageForTesting())

indentation is wrong

> Source/WebKit/WebProcess/GPU/GPUProcessConnection.cpp:133
> +            instance->connection().setIgnoreInvalidMessageForTesting();

ditto.

> Source/WebKit/WebProcess/GPU/GPUProcessConnection.cpp:306
> +    WTFLogAlways("Got didinitialize");

Should drop.
Comment 6 Kimmo Kinnunen 2022-05-05 01:32:23 PDT 
Created attachment 458858 [details]
For landing
Comment 7 Kimmo Kinnunen 2022-05-05 02:42:51 PDT 
(In reply to Chris Dumez from comment #5)
> Comment on attachment 458801 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=458801&action=review
> 
> r=me with fixes. We may want to do the same for the network process?

Thanks!

I think network process, authn process and plugin process could all use this pattern.

However, I see the network process startup seems to be a bit intricate too, given the retries (?) and all. Maybe if there's no immediate need we should do it a bit later? Maybe after it is apparent that this change didn't cause any unforeseen issues.
Comment 8 EWS 2022-05-05 06:43:49 PDT 
Committed r293829 (250302@main): <https://commits.webkit.org/250302@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 458858 [details].