[PATCH 4/4] client: fix bug with display->reader_count < 0
Marek Chalupa
mchqwerty at gmail.com
Fri Aug 29 02:21:31 PDT 2014
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 <mchqwerty at gmail.com>
---
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
More information about the wayland-devel
mailing list