[PATCH 2/2] client: wake up sleeping threads on all return paths from read_events

Marek Chalupa mchqwerty at gmail.com
Thu Aug 28 02:32:56 PDT 2014


In the first place, create a function display_wakeup_threads that
contains always-repeated pattern: broadcast + increase serial.

Replace the pattern with this function and call this function also
on return from read_events when wl_connection_read fails with
EAGAIN (this solves the bug from the test from previous commit).

Now all return paths from read_events are safe and wake up the threads.
There's a path in wl_display_read_events that do not wake up
threads - this is the case when last_error is set. But on this
path are the threads always awake, because every function that can
set last_error sets it via display_fatal_error (which calls
display_wakeup_threads).

Signed-off-by: Marek Chalupa <mchqwerty at gmail.com>
---
 src/wayland-client.c | 42 +++++++++++++++++++++++++++++-------------
 1 file changed, 29 insertions(+), 13 deletions(-)

diff --git a/src/wayland-client.c b/src/wayland-client.c
index 9159ee0..9574cf7 100644
--- a/src/wayland-client.c
+++ b/src/wayland-client.c
@@ -110,6 +110,27 @@ struct wl_display {
 static int debug_client = 0;
 
 /**
+ * This helper function wakes up all threads that are
+ * waiting for display->reader_cond (i. e. when reading is done
+ * canceled, or an error occured)
+ *
+ * NOTE: must be called with display->mutex locked
+ */
+static void
+display_wakeup_threads(struct wl_display *display)
+{
+       pthread_cond_broadcast(&display->reader_cond);
+
+	/* Thread can get sleeping only in read_events() and if we're
+	* waking it up, it means that the read processed or was
+	* canceled, so we must increase the read_serial.
+	* This prevents from indefinite looping in read_events()
+	* (when calling pthread_cond_wait under condition
+	* display->read_serial == serial). */
+       ++display->read_serial;
+}
+
+/**
  * This function is called for local errors (no memory, server hung up)
  *
  * \param display
@@ -128,11 +149,7 @@ display_fatal_error(struct wl_display *display, int error)
 
 	display->last_error = error;
 
-       pthread_cond_broadcast(&display->reader_cond);
-       /* prevent from indefinite looping in read_events()
-	* (when calling pthread_cond_wait under condition
-	* display->read_serial == serial) */
-       ++display->read_serial;
+	display_wakeup_threads(display);
 }
 
 /**
@@ -182,7 +199,7 @@ display_protocol_error(struct wl_display *display, uint32_t code,
 	display->protocol_error.interface = intf;
 
 	/*
-	 * here it is not necessary to broadcast reader's cond like in
+	 * here it is not necessary to wake up threads like in
 	 * display_fatal_error, because this function is called from
 	 * an event handler and that means that read_events() is done
 	 * and woke up all threads. Since wl_display_prepare_read()
@@ -1138,8 +1155,10 @@ 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) {
+				display_wakeup_threads(display);
 				return 0;
+			}
 
 			display_fatal_error(display, errno);
 			return -1;
@@ -1162,8 +1181,7 @@ read_events(struct wl_display *display)
 			}
 		}
 
-		display->read_serial++;
-		pthread_cond_broadcast(&display->reader_cond);
+		display_wakeup_threads(display);
 	} else {
 		serial = display->read_serial;
 		while (display->read_serial == serial)
@@ -1348,10 +1366,8 @@ wl_display_cancel_read(struct wl_display *display)
 	pthread_mutex_lock(&display->mutex);
 
 	display->reader_count--;
-	if (display->reader_count == 0) {
-		display->read_serial++;
-		pthread_cond_broadcast(&display->reader_cond);
-	}
+	if (display->reader_count == 0)
+		display_wakeup_threads(display);
 
 	pthread_mutex_unlock(&display->mutex);
 }
-- 
1.9.3



More information about the wayland-devel mailing list