[PATCH] event-loop: fix blocking timerfd read

[Kristian Høgsberg]

On Thu, Oct 17, 2013 at 02:20:31PM +0200, David Herrmann wrote:
> If we call wl_event_source_check() on a timerfd, our dispatcher will block
> on the timerfd read(). Avoid calling read() if no epoll-event was
> reported.
> 
> Note: The internal tick-count of a timerfd is never decreased. So once we
> get signaled a read event, a following read() is guaranteed to succeed
> _if_ no other operation is called on the timerfd in between. Therefore,
> there is no need for us to make the read() non-blocking (even with
> clock-changes we never block). However, to be safe for future changed,
> just ignore EAGAIN safely.
> 
> Also remove the comment about read() failure. The only reasons the read
> can currently fail is EINTR or invalid parameters. The latter cannot
> happen in our setup so ignore it. EINTR shouldn't happen as
> syscall-restart is the default behavior so we simply return if a user
> changed it. Better safe than sorry.

We should never need to check timer or signal sources.  The checked
source are only for things like xlib or wayland-client where we may
read events into an event queue and empty the fd.  Then when we go to
sleep we have events pending, but the fd is not readable.  The "check"
is there to let us check whether those sources have events to dispatch
before going to sleep.

If anything, maybe we should just fail if somebody calls
wl_event_source_check() on a timer or signal source.

Kristian

> Signed-off-by: David Herrmann <dh.herrmann at gmail.com>
> ---
>  src/event-loop.c | 13 +++++++++----
>  1 file changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/src/event-loop.c b/src/event-loop.c
> index d323601..d340e3c 100644
> --- a/src/event-loop.c
> +++ b/src/event-loop.c
> @@ -172,10 +172,15 @@ wl_event_source_timer_dispatch(struct wl_event_source *source,
>  	uint64_t expires;
>  	int len;
>  
> -	len = read(source->fd, &expires, sizeof expires);
> -	if (len != sizeof expires)
> -		/* Is there anything we can do here?  Will this ever happen? */
> -		fprintf(stderr, "timerfd read error: %m\n");
> +	if (ep->events != 0) {
> +		len = read(source->fd, &expires, sizeof expires);
> +		if (len != sizeof expires) {
> +			if (len >= 0 || (errno != EINTR && errno != EAGAIN))
> +				fprintf(stderr,
> +					"timerfd read error (%d): %m\n", len);
> +			return 0;
> +		}
> +	}
>  
>  	return timer_source->func(timer_source->base.data);
>  }
> -- 
> 1.8.4
>