[PATCH] client: Add acquire-fd API to avoid requiring a polling main thread

[Uli Schlachter]

On 10.04.2013 17:16, Kristian Høgsberg wrote:
[...]
> +WL_EXPORT int 
> +wl_display_acquire_fd(struct wl_display *display)
> +{
> +	char c = 0;
> +	int old_state;
> +
> +	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;
> +	}
> +
> +	old_state = display->reader_state;
> +	display->reader_state = LOCKED_READER;
> +	display->reader = pthread_self();
> +
> +	if (old_state == VOLUNTEER_READER) {
> +		write(display->reader_pipe[1], &c, 1);
> +		pthread_cond_wait(&display->pipe_cond, &display->mutex);
> +	}
> +
> +	pthread_mutex_unlock(&display->mutex);
> +
> +	return display->fd;
> +}
[...]
> +	if (display->reader_state == NO_READER) {
> +		/* A thread gets here when it's the first reader to
> +		 * try to read and there's no LOCKED_READER.  We set
> +		 * VOLUNTEER_READER and poll on the fd and the
> +		 * reader_pipe, so that a locked reader can preempt if
> +		 * it needs to. */
> +		display->reader = pthread_self();
> +		display->reader_state = VOLUNTEER_READER;
> +
> +		pthread_mutex_unlock(&display->mutex);
> +
> +		pfd[0].fd = display->fd;
> +		pfd[0].events = POLLIN;
> +		pfd[1].fd = display->reader_pipe[0];
> +		pfd[1].events = POLLIN;
> +		ret = poll(pfd, 2, -1);
> +		if (pfd[1].revents & POLLIN)
> +			read(display->reader_pipe[0], &c, 1);
> +
> +		pthread_mutex_lock(&display->mutex);
> +
> +		pthread_cond_signal(&display->pipe_cond);
> +
> +		if (ret == -1) {
> +			display_fatal_error(display, errno);
> +			return -1;
> +		}
[...]

This still has the "pending byte in the pipe"-problem from earlier, doesn't it?

I mean this one:

Uli Schlachter wrote:
> Ok, I think I found another problem:
> 
> - Thread 1 is a volunteer reader, sitting in poll()
> - The server sends some data and poll() returns because display->fd is readable
> - Thread 2 calls wl_display_acquire_fd() and reaches the cond_wait(pipe_cond)
> - Thread 1 reaches the above cond_wait(reader_cond)
> - Thread 2 normally reads from the connection and wakes up all readers.
> 
> At this point, everything is OK. No thread has any problems. However, there is
> still a single byte sitting in the pipe that was not read. So what happens next?
> 
> - Thread X wants to read from the display and becomes a volunteer reader. Since
> the pipe is readable, it immediately continues, reads the byte and ends up in
> the above cond_wait(reader_cond). At this point we have a thread waiting on
> reader_cond, but no reader which will wake it up (=exactly the situation that
> inspired this patch, just a lot more unlikely).

Cheers,
Uli

P.S.: Urgh, I really don't like pthread's spurious wakeups. Thanks for
remembering them, Jiergir.
-- 
Q: Because it reverses the logical flow of conversation.
A: Why is putting a reply at the top of the message frowned upon?