On Fri, 29 Aug 2014 11:21:30 +0200 Marek Chalupa <[email protected]> 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 <[email protected]> > --- > tests/display-test.c | 88 > ++++++++++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 88 insertions(+) > > diff --git a/tests/display-test.c b/tests/display-test.c > index 1289866..e1cf830 100644 > --- a/tests/display-test.c > +++ b/tests/display-test.c > @@ -32,6 +32,7 @@ > #include <sys/types.h> > #include <sys/stat.h> > #include <pthread.h> > +#include <poll.h> > > #include "wayland-private.h" > #include "wayland-server.h" > @@ -480,6 +481,93 @@ TEST(threading_cancel_read_tst) > display_destroy(d); > } > > +/* set this to 1 when it is right that > + * the thread is woken up */ > +static int threads_can_be_woken_up = 0; > + > +static void * > +thread_eagain_func(void *data) { > + struct client *c = data; > + > + register_reading(c->wl_display); > + > + c->display_stopped = 1; > + assert(wl_display_read_events(c->wl_display) == 0 > + && errno != EAGAIN); > + /* if thread was sleeping in read_events, this assert > + * must pass now */ > + assert(threads_can_be_woken_up && "Thread woke up too early"); > + > + pthread_exit(NULL); > +} > + > +static void > +threading_read_eagain(void) > +{ > + struct client *c = client_connect(); > + pthread_t th1, th2, th3; > + struct pollfd fds; > + int i; > + > + register_reading(c->wl_display); > + > + th1 = create_thread(c, thread_eagain_func); > + th2 = create_thread(c, thread_eagain_func); > + th3 = create_thread(c, thread_eagain_func); > + > + /* All the threads are sleeping, waiting until read or cancel > + * is called. Since we have no data on socket waiting, > + * the wl_connection_read should end up with error and set errno > + * to EAGAIN => wl_display_read_events() will return 0 and set errno > + * to EAGAIN. It should not wake up the sleeping threads, so that > + * this thread can try to read again */ > + alarm(3); > + assert(wl_display_read_events(c->wl_display) == 0); > + assert(errno == EAGAIN); I think there is a problem here. wl_display_read_events() returning 0 means success, and in that case errno may not be set if it was a real success. There is no EAGAIN to be handled, it's already done inside read_events(). In this test it won't be a real success, but you cannot code clients to check for errno after getting 0 from wl_display_read_events(). > + > + /* the same should happen next times - but there was a bug when > + * the main thread got blocked! */ > + for (i = 0; i < 5; ++i) { > + assert(wl_display_read_events(c->wl_display) == 0); Therefore you are here calling wl_display_read_events() again after it succeeded! And that IMO is a developer error. Besides, I cannot see anywhere where it says that it is ok to call wl_display_read_events() again even if it failed, without calling wl_display_prepare_read() first. 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. > + assert(errno == EAGAIN); > + } > + > + /* generate something to read and wait until the event comes */ > + wl_proxy_marshal((struct wl_proxy *) c->tc, > + 0 /* STOP DISPLAY */, 1 /* number */); > + assert(wl_display_flush(c->wl_display) > 0); > + > + fds.fd = wl_display_get_fd(c->wl_display); > + fds.events = POLLIN; > + assert(poll(&fds, 1, 3) == 1); Is timeout of 3 milliseconds intended? > + > + /* threads can be woken up now */ > + threads_can_be_woken_up = 1; > + > + errno = 0; > + assert(wl_display_read_events(c->wl_display) == 0); > + assert(errno != EAGAIN); > + assert(wl_display_dispatch_pending(c->wl_display) == 1); > + > + pthread_join(th1, NULL); > + pthread_join(th2, NULL); > + pthread_join(th3, NULL); > + > + client_disconnect(c); > +} > + > +TEST(threading_read_eagain_tst) > +{ > + struct display *d = display_create(); > + > + client_create(d, threading_read_eagain); > + > + display_run(d); > + display_resume(d); > + > + display_destroy(d); > +} > + > static void * > thread_prepare_and_read2(void *data) > { Thanks, pq _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
