|
Hi Derek >> 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.
I agree that we can easily find a client side problem that consumes events
too slowly. I wonder why we don't need to have EAGAIN handling logic in
wl_closure_send(). As mentioned above, there is already an error handling
logic of EAGAIN in wl_display_flush_clients(). Even if there is an error on
flushing to a client in wl_display_flush_clients(), we'll have the next
chance to flush to the client. I think it is necessary to make it clear in
terms of consistency.
> 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.
Agreed. Dropping events will cause many problems.
By the way, when it failed to flush event(s) to a client, the event(s)
will be dropped by default as wayland server library doesn't care about it.
That's what I wanted to talk about.
> What if it's a frame event that gets dropped and not an input event? Or
> a resource destroy?
Instead of destroying a connection, destroying a resource seems to be more
reasonable.
> 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.
Of course, in many case, optimization must be done in client side and we're
also doing many optimizations in client side and they're making the situation
better. But I think it's not enough. IMO, we need to think about
what's the proper value for the buffer size of wl_connection's in/out in
wayland client and what's the effect of shrink/expand the size of the buffer.
Thanks
Sung-Jin
> Signed-off-by: jeon <[email protected]
> <mailto:[email protected]>>
>
> ---
> 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
>
>
> _______________________________________________
> wayland-devel mailing list
> [email protected]
> https://lists.freedesktop.org/mailman/listinfo/wayland-devel
>
_______________________________________________
wayland-devel mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/wayland-devel
![]() |
