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);

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 <[email protected]>
---
 src/wayland-client.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/src/wayland-client.c b/src/wayland-client.c
index 4fc7105..6f806f8 100644
--- a/src/wayland-client.c
+++ b/src/wayland-client.c
@@ -1154,8 +1154,15 @@ read_events(struct wl_display *display)
        if (display->reader_count == 0) {
                total = wl_connection_read(display->connection);
                if (total == -1) {
-                       if (errno == EAGAIN)
+                       if (errno == EAGAIN) {
+                       /* do not wake up threads, so that the thread that
+                        * is lucky to read can try to read again. We just
+                        * need to increase display->reader_count to 1 again,
+                        * so that in the next trial, the thread will get
+                        * to this branch again */
+                               ++display->reader_count;
                                return 0;
+                       }
 
                        display_fatal_error(display, errno);
                        return -1;
@@ -1179,11 +1186,13 @@ read_events(struct wl_display *display)
                }
 
                display_wakeup_threads(display);
-       } else {
+       } else if (display->reader_count > 0) {
                serial = display->read_serial;
                while (display->read_serial == serial)
                        pthread_cond_wait(&display->reader_cond,
                                          &display->mutex);
+       } else {
+               assert(0 && "display->reader_count should never be negative");
        }
 
        return 0;
-- 
1.9.3

_______________________________________________
wayland-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/wayland-devel

Reply via email to