[PATCH] client: broadcast the right pthread_cond variable

[Pekka Paalanen]

On Tue,  5 Aug 2014 11:42:01 +0200
Marek Chalupa <mchqwerty at gmail.com> wrote:

> In previous commit we removed unused variables. One of them was
> pthread_cond_t that was formerly used when reading from display, but
> later was (erroneously) made unused. This patch fixes this error
> and is a fix for the failing test introduced few patches ago (tests:
> test if thread can block on error)
> ---
>  src/wayland-client.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
> 
> diff --git a/src/wayland-client.c b/src/wayland-client.c
> index 844138b..68a91f6 100644
> --- a/src/wayland-client.c
> +++ b/src/wayland-client.c
> @@ -129,6 +129,12 @@ display_fatal_error(struct wl_display *display, int error)
>  		error = EFAULT;
>  
>  	display->last_error = error;
> +
> +       pthread_cond_broadcast(&display->reader_cond);
> +       /* prevent from indefinite looping in read_events()
> +	* (when calling pthread_cond_wait under condition
> +	* display->read_serial == serial) */
> +       ++display->read_serial;
>  }
>  
>  /**
> @@ -177,6 +183,16 @@ display_protocol_error(struct wl_display *display, uint32_t code,
>  	display->protocol_error.id = id;
>  	display->protocol_error.interface = intf;
>  
> +	/*
> +	 * here it is not necessary to broadcast reader's cond like in
> +	 * display_fatal_error, because this function is called from
> +	 * an event handler and that means that read_events() is done
> +	 * and woke up all threads. Since wl_display_prepare_read()
> +	 * fails when there are events in the queue, no threads
> +	 * can sleep in read_events() during dispatching
> +	 * (and therefore during calling this function), so this is safe.
> +	 */

I have been staring at this explanation for a while. Tell me if this
scenario is impossible:

There are two threads, both running an event loop with prepare_read and
their own event queues, as usual. An error comes in via the private
display_queue and we end up here this thread. Dispatching can happen in
parallel in different queues. The other thread dispatches, goes to
prepare_read and blocks while we hold the mutex. We release the mutex
and return, wl_display is now in error state. The other thread
continues(?) to wl_display_flush() and gets an error... hey, that works.

If it continues directly to wl_display_read_events()... we have added
(will add) the last_error check in there, and it'll return failure. That
works too.

Ok, I can't imagine a case where this fails, so I'm committing this. It
does fix the test. :-)

> +
>  	pthread_mutex_unlock(&display->mutex);
>  }
>  

This and
[PATCH v3] client: drop unused event queue cond and list variables
pushed.


Thanks,
pq