Bug 253791

Summary: `RemoteRenderingBackendProxy` should send all requests asynchronously instead of synchronously
Product: WebKit Reporter: Darren Mo <darrenmo>
Component: WebKit2Assignee: Nobody <webkit-unassigned>
Status: NEW    
Severity: Normal CC: cdumez, empire_utopian.0v, kkinnunen, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=239905
https://bugs.webkit.org/show_bug.cgi?id=243838
Attachments:
Description Flags
A sample project demonstrating a deadlock that is possible due to this issue. none

Darren Mo
Reported 2023-03-12 00:54:23 PST
Created attachment 465401 [details] A sample project demonstrating a deadlock that is possible due to this issue. `RemoteRenderingBackendProxy` sends requests to the GPU process. At the time of writing, there are 19 requests, 5 of which are synchronous. If possible, those 5 requests should be made asynchronous to avoid deadlock. Deadlock is possible between the Web process and the GPU process if the GPU process sends a synchronous request to the Web process. (I haven’t looked into whether the GPU process makes any such requests to the Web process.) Deadlock is possible between the Web process and the UI process when the connection to the GPU process has not yet been established since the UI process facilitates the connection establishment. This was partially addressed in https://bugs.webkit.org/show_bug.cgi?id=239905 by making it so that the Web process creates the connection and sends the connection details asynchronously to the UI process. However, the deadlock is still possible if the Web process synchronously makes a request to the GPU process before the UI process has finished facilitating the connection between the Web process and the GPU process. See the attached sample project for a minimal reproduction of the issue.
Attachments
A sample project demonstrating a deadlock that is possible due to this issue. (24.05 KB, application/zip)
2023-03-12 00:54 PST, Darren Mo 		no flags
Details
View All Add attachment proposed patch, testcase, etc.
Kimmo Kinnunen
Comment 1 2023-03-13 11:35:15 PDT
Thanks for the report and especially of the repro case. I'll take a look in the near future. > `RemoteRenderingBackendProxy` sends requests to the GPU process. At the time of writing, there are 19 requests, 5 of which are synchronous. If possible, those 5 requests should be made asynchronous to avoid deadlock. I think in general this is not the intention. Some GPU related operations are synchronous, like GL calls and such. For the time being these are intended to be synchronous, so the solution needs to account for these. > Deadlock is possible between the Web process and the GPU process if the GPU process sends a synchronous request to the Web process. (I haven’t looked into whether the GPU process makes any such requests to the Web process.) The design intention is that GPU process doesn't send synchronously to Web process. > Deadlock is possible between the Web process and the UI process when the connection to the GPU process has not yet been established since the UI process facilitates the connection establishment. There are couple "rare" cases of plain WEB -> UI, UI -> WEB deadlock scenarios that we have not yet solved. It's unclear if you're running into these or if it's a different failure scenario. > However, the deadlock is still possible if the Web process synchronously makes a request to the GPU process before the UI process has finished facilitating the connection between the Web process and the GPU process. The intention of the linked bug was to mostly address these. It may well be that it did not catch all cases, I'll check your repro case.
Darren Mo
Comment 2 2023-03-13 15:54:18 PDT
> Thanks for the report and especially of the repro case. I'll take a look in > the near future. Thanks for looking into this, Kimmo! > > `RemoteRenderingBackendProxy` sends requests to the GPU process. At the time of writing, there are 19 requests, 5 of which are synchronous. If possible, those 5 requests should be made asynchronous to avoid deadlock. > > I think in general this is not the intention. Some GPU related operations > are synchronous, like GL calls and such. For the time being these are > intended to be synchronous, so the solution needs to account for these. Hmm I guess that makes sense. One idea that came to my mind: could we establish the connection to the GPU process at the start of the Web process before any rendering happens? > The design intention is that GPU process doesn't send synchronously to Web > process. Great! > The intention of the linked bug was to mostly address these. It may well be > that it did not catch all cases, I'll check your repro case. Thanks! Some comments regarding the attached sample project: - It deadlocks only on iOS 16+ because the “GPU Process: DOM Rendering” feature flag is enabled by default on iOS 16. - It uses an anti-pattern to trigger the deadlock: it synchronously waits for `evaluateJavaScript:completionHandler:` to complete. This is just for reproduction purposes; I’m sure one can imagine other possible synchronous calls that would have a similar effect. (That being said, this anti-pattern was found in a popular app owned by a very big company. 😬🤫)
Radar WebKit Bug Importer
Comment 3 2023-03-19 01:55:13 PDT
<rdar://problem/106909481>
Note You need to log in before you can comment on or make changes to this bug.