[Webkit-unassigned] [Bug 232580] [GPU Process] Rendering backend may stop processing IPC stream messages after a sync message timeout

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=232580

Wenson Hsieh <wenson_hsieh at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |cdumez at apple.com,
                   |                            |thorton at apple.com

--- Comment #4 from Wenson Hsieh <wenson_hsieh at apple.com> ---
(In reply to Wenson Hsieh from comment #3)
> (In reply to Kimmo Kinnunen from comment #2)
> > Yeah.
> > The design is so that timeouts do not make sense.
> > 
> > If WP asks to draw a 4k x 4k image, and that never completes, there's
> > nothing sensible you can do except restart the GPU process.
> > 
> > The rationale:
> > We define timeout as X seconds, e.g. if message is not replied within X
> > seconds, we declare the GPU process has hung.
> > 
> > You send message.
> > The message times out.
> > What do you do now?
> > 
> > A) Maybe GPUP has not hung, we should be able to retry something like 3 times
> >   - If the GPUP has not hung, why did you define "GPUP is hung if timeout X
> > seconds"?
> >   - I still want to retry 3 times
> >   - Why did you not define "GPUP is hung if timeout is 3*X seconds"? What
> > does the 
> >     retry contribute here?
> >   - ...
> 
> Agreed! Implementing some kind of retry policy is redundant with timeouts in
> the first place, since the timeout is arbitrary anyways.
> 
> > 
> > B) We did define X sec timeout means GPU process has hung, so GPU process
> > has hung. Restart it.
> 
> That said, it's critical that a web process does not have the capability to
> cause the GPU Process to "restart" arbitrarily, as the client is untrusted.
> We wouldn't want a compromised web process to be able to effectively kill
> and relaunch the GPU Process in a loop.
> 
> Now that I think about it more, I believe there's a third option (C) here,
> which is to simply remove the timeout and just use Seconds::infinity() for
> these sync messages. The client (web process) trusts the server (GPU
> process) to be able to handle incoming drawing commands, and knows that it's
> only a matter of time until the GPU Process either:
>     [1] ..finishes executing the command and sends the reply, in which case
> the `sendSync()` succeeds or
>     [2] ..jetsams or crashes in the process, in which case the `sendSync()`
> fails and we just carry on in the web process

I should also note — using an ∞ timeout is also how most of the regular `IPC::Connection::sendSync()` calls from the web process to the UI process work, and the trust model between the web and UI process is pretty much the same as the trust model between the web and GPU process.

The notion of a sync IPC timeout is really only useful in the other direction (UI -> web, or more generally "trusted" -> "untrusted"), since any sync IPC in this direction (1) will prevent the trusted process from doing anything else in the meantime, and (2) is not guaranteed to complete in a timely manner.

> 
> > 
> > 
> > "Retry" or "carry on like nothing happened and hope for the best" is ok if
> > the medium is such that it's expected that messages get lost, like a
> > network. IPC isn't like that, nothing should be lost by design.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20211102/c887ece7/attachment.htm>