[PATCH wayland] Use non-blocking timerfd to prevent blocking when updating timer event sources

[Kristian Høgsberg]

On Fri, Apr 25, 2014 at 03:00:54PM +0100, Andrew Wedgbury wrote:
> This implements a simple fix for the blocking problem that occurs when 
> updating a timer event source after the timer expires, but before its 
> callback is dispatched. This can happen when another event happens during the
> same epoll wakeup as the timer event, and causes the read() call in 
> wl_event_source_timer_dispatch() to block for the updated duration of the 
> timer.
> 
> We never want this read() call to block, so I believe it makes sense for the
> timerfd to be non-blocking, and we simply ignore the case where the read fails
> with EAGAIN. We still report all other errors as before, and still ignore the
> actual value read from the socket.
> 
> With this change, the event_loop_timer_updates unit test case I submitted
> previously now passes, and weston appears to work as before.

Thanks, Andrew, good test case and analysis and I agree with the fix.
Test case and fix committed.

Kristian

> 
> ---
>  src/event-loop.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/src/event-loop.c b/src/event-loop.c
> index d323601..1046600 100644
> --- a/src/event-loop.c
> +++ b/src/event-loop.c
> @@ -173,7 +173,7 @@ wl_event_source_timer_dispatch(struct wl_event_source *source,
>  	int len;
>  
>  	len = read(source->fd, &expires, sizeof expires);
> -	if (len != sizeof expires)
> +	if (!(len == -1 && errno == EAGAIN) && len != sizeof expires)
>  		/* Is there anything we can do here?  Will this ever happen? */
>  		fprintf(stderr, "timerfd read error: %m\n");
>  
> @@ -196,7 +196,8 @@ wl_event_loop_add_timer(struct wl_event_loop *loop,
>  		return NULL;
>  
>  	source->base.interface = &timer_source_interface;
> -	source->base.fd = timerfd_create(CLOCK_MONOTONIC, TFD_CLOEXEC);
> +	source->base.fd = timerfd_create(CLOCK_MONOTONIC,
> +									 TFD_CLOEXEC | TFD_NONBLOCK);
>  	source->func = func;
>  
>  	return add_source(loop, &source->base, WL_EVENT_READABLE, data);
> -- 
> 1.9.2
> 
> _______________________________________________
> wayland-devel mailing list
> wayland-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/wayland-devel