[PATCH 3/4] tests: add display test that highlights another bug in read_events

[Pekka Paalanen]

On Tue, 9 Sep 2014 15:39:35 +0200
Marek Chalupa <mchqwerty at gmail.com> wrote:

> On 9 September 2014 13:10, Marek Chalupa <mchqwerty at gmail.com> wrote:
> 
> >
> >
> > On 4 September 2014 15:17, Pekka Paalanen <ppaalanen at gmail.com>
> > wrote:
> >
> >> On Fri, 29 Aug 2014 11:21:30 +0200
> >> Marek Chalupa <mchqwerty at gmail.com> wrote:
> >>
> >> > When wl_display_read_events() returns with errno == EAGAIN, we
> >> > naturally try to call it again. But this next call results in
> >> > deadlock.
> >> >
> >> > Signed-off-by: Marek Chalupa <mchqwerty at gmail.com>
> >> > ---
> >> >  tests/display-test.c | 88
> >> ++++++++++++++++++++++++++++++++++++++++++++++++++++
> >> >  1 file changed, 88 insertions(+)

> >> But I see something else: wl_display_read_events() does not
> >> decrement reader_count if it returns due to last_error. I think
> >> that needs a fix to guarantee the reader_count is decremented, so
> >> that the side-effects are consistent.
> >>
> >
> > Yes, we should fix this. There are cases that can cause a problem.
> >
> 
> Actually, I can not make up any use-case where it is problem... do
> you have any?

No, I just assumed it could be a problem, since it looks like it would
violate an invariant (keeping the reader count consistent) by
returning without decrementing. After all, prepare_read already
succeeded, so it incremented the reader count.

I suppose the safest approach would be to maintain the guarantees we
already had in 1.5.0.

Should a read_event failing due to last_error wake up all threads? It
should be completely safe, but OTOH every site setting last_error
already wakes up all threads and prevents anyone from sleeping in
read_events again, right?


Thanks,
pq