[PATCH 4/4] client: fix bug with display->reader_count < 0

[Pekka Paalanen]

On Fri, 29 Aug 2014 11:21:31 +0200
Marek Chalupa <mchqwerty at gmail.com> wrote:

> If we will try call wl_display_read_events() again,
> after we got EAGAIN from previous call, we get deadlock
> as shown in test. The bug works like this: after first call
> to wl_display_read_events() the display->reader_count is 0
> and next call will decrease it to -1 so the thread will make
> sleeping itself and we have a deadlock.
> 
> We have two options here:
> 
> Either we can wake up all threads before returning from
> read_events() and thus force the programmer to do the whole
> prepare+read loop again. Waking up all the threads could bring
> unnecessary overhead.
> 
> The other solution, that I chose, is to return without waking up
> the threads, so that programmer can try read again
> - it feels natural after getting EAGAIN. Thus the proper use
> of wl_display_read_events should look like:
> 
>     do {
>         ret = wl_display_read_events(display);
>     } while (ret == 0 && errno == EAGAIN);

According to my understanding, this is just not the way how to use
wl_display_read_events(), so NAK for patches 3 and 4. Explanation is in
my reply to patch 3.

Patches 1 and 2 look nice, so those are pushed.


Thanks,
pq


> 
> The solution is simple. Before returning from read_events under
> these circumstances, increase the display->reader_count back to 1,
> so that next trial will have the same execution path as the one
> before.
> 
> Signed-off-by: Marek Chalupa <mchqwerty at gmail.com>
> ---
>  src/wayland-client.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)