[PATCH] wayland-server: update the client fd when it failed to flush with EAGAIN

[Pekka Paalanen]

On Mon, 26 Nov 2018 11:12:08 -0600
Derek Foreman <derek.foreman.samsung at gmail.com> wrote:

> On 11/22/18 11:08 PM, Jeonghyun Kang wrote:
> > When a wayland compositor gets an EAGAIN error whenever
> > sending or receiving event(s) to a client in the
> > wl_closure_send() or the wl_closure_queue(), the error
> > variable of the wl_client for the client will be set to
> > true and the client is going to be killed by the
> > compositor later soon. This patch fixes the problem by
> > updating the socket fd's event mask as we're doing
> > in wl_display_flush_clients() for having another chance
> > to do it again.
> > 
> > Actually, this kind of problem can be watched in an
> > environment in which massive input events are coming
> > from multi-touch screen or multi-pen devices. In this
> > kind of environment, a client receiving the massive
> > input events are trying to drawing something very hard
> > but it's being killed sooner or later. With the given
> > patch, the client receiving the input events is not being
> > destroyed and is working well even though it doesn't get
> > the whole input events from the compositor.  
> 
> This is actually intentional behavior to avoid infinitely growing send
> queues for clients that process events too slowly.

Indeed. If a client cannot keep up with the compositor because the
client is doing something else, the client is by definition not
responsive enough and need to be fixed. It may also be a compositor bug
attempting to send too much data to a client, in which case the
compositor or even the protocol design will need changes to reduce the
volume.

> I don't think dropping events randomly (or at all) because the client
> can't keep up is something we should do in libwayland. It'll just make
> for nearly impossible to reproduce and debug problems when client and
> compositor state fall out of sync.

Yes, dropping events is never acceptable.

The whole Wayland protocol design relies on the fact that no message is
*ever* dropped. The fundamental protocol design also relies on all
messages being in order, which means that re-ordering is as bad as
dropping events.

If there is any situation where a message would need to be dropped, it
is much more merciful to just disconnect the client, because otherwise
the server and client would get out of sync, which just causes more
problems that are extremely painful to debug.

> 
> What if it's a frame event that gets dropped and not an input event?  Or
> a resource destroy?
> 
> The disconnect is a clear indication that there's a problem that needs
> to be fixed somewhere else. The client needs to process events in a
> timely fashion, or the compositor needs to send less events.

Exactly.

NAK for the patch.


Thanks,
pq

> 
> Thanks,
> Derek
> 
> > Signed-off-by: jeon <jhyuni.kang at samsung.com
> > <mailto:jhyuni.kang at samsung.com>>
> > 
> > ---
> >  src/wayland-server.c | 11 +++++++++--
> >  1 file changed, 9 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/wayland-server.c b/src/wayland-server.c
> > index eae8d2e..5afaa28 100644
> > --- a/src/wayland-server.c
> > +++ b/src/wayland-server.c
> > @@ -222,8 +222,15 @@ handle_array(struct wl_resource *resource, uint32_t
> > opcode,
> >  
> >         log_closure(resource, closure, true);
> >  
> > -       if (send_func(closure, resource->client->connection))
> > -               resource->client->error = 1;
> > +       if (send_func(closure, resource->client->connection)) {
> > +               if (errno == EAGAIN) {
> > +                       wl_event_source_fd_update(resource->client->source,
> > +                                                 WL_EVENT_WRITABLE |
> > +                                                 WL_EVENT_READABLE);
> > +               } else {
> > +                       resource->client->error = 1;
> > +               }
> > +       }
> >  
> >         wl_closure_destroy(closure);
> >  }
> > 
> > -- 
> > 2.7.4
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20181127/cf87b072/attachment.sig>