[PATCH] client: Add acquire-fd API to avoid requiring a polling main thread
Uli Schlachter
psychon at znc.in
Thu Apr 11 00:53:58 PDT 2013
On 10.04.2013 23:55, Kristian Høgsberg wrote:
[...]
> +WL_EXPORT int
> +wl_display_acquire_fd(struct wl_display *display)
> +{
> + char c = 0;
> +
> + pthread_mutex_lock(&display->mutex);
> +
> + if (display->reader_state == LOCKED_READER &&
> + !pthread_equal(display->reader, pthread_self())) {
> + errno = EBUSY;
> + pthread_mutex_unlock(&display->mutex);
> + return -1;
> + }
> +
> + pthread_cond_signal(&display->pipe_cond);
I feel light I am missing something, so: Why does this call cond_signal() here?
The only threads which might be waiting on this condition are waiting for the
volunteer reader to exit and they will not be able to progress until that one
unsets pipe_busy. So the signal() here won't do anything bad, but also none of
the threads waking up will be able to do anything.
> + if (display->reader_state == VOLUNTEER_READER)
> + write(display->reader_pipe[1], &c, 1);
> +
> + display->reader = pthread_self();
> + display->reader_state = LOCKED_READER;
> +
> + pthread_mutex_unlock(&display->mutex);
> +
> + return display->fd;
> +}
[...]
> + pthread_mutex_lock(&display->mutex);
> +
> + display->pipe_busy = 0;
> + if (display->reader_state != VOLUNTEER_READER) {
> + read(display->reader_pipe[0], &c, 1);
> + pthread_cond_signal(&display->pipe_cond);
Why is this a cond_signal()? If more than one thread is waiting on pipe_cond,
all of them should be waken up. One of them will/might become a volunteer
reader, the others should end up waiting on reader_cond.
(Otherwise the volunteer reader might read something useful for one of the
threads, but this thread is still waiting on pipe_cond).
> + }
>
> - ret = wl_connection_flush(display->connection);
> - if (ret < 0 && errno != EAGAIN) {
> - display_fatal_error(display, errno);
> - goto err_unlock;
> + if (ret == -1) {
> + display_fatal_error(display, errno);
> + return -1;
> + }
> }
[...]
The synchronization in this patch looks solid to me. I will think about it a
little more, but I didn't yet manage to really break it. Yay. :-)
Cheers,
Uli
More information about the wayland-devel
mailing list