[PATCH] client: use the thread reader_count to fix deadlock.

Boram Park boram1288.park at samsung.com
Thu Apr 7 23:58:53 UTC 2016


This patch solves the deadlock issue of the below scenario.

  1. A thread polls several fds including the wayland's fd.
  2. This thread probably calls wl_display_prepare_read() before polling
     fds.
  3. This thread could be awake by other event source which isn't related
     with wayland fd.
  4. After wake up, this thread could call wl_display_dispatch or
     wl_display_roundtrip for sync operation.
  5. Then, when this thread got a done event. this thread will stuck in
     deadlock because this thread increases +2 reader_count in the same
     thread.

The read_event or cancel_read for the first prepare_read is not going to
happen because this thread sleeps by pthread_cond_wait() of read_event.

This problem can be solved by using the reader_count per thread.

The thread reader_count will be increased/decreased whenever prepare_read,
cancel_read and read_event are called.

However, the original display reader_count will be increased only once per
thread. And, when cancel_read and read_event are called, it will be
decreased for this thread to read event from fd and wake up other threads.
After that, if the thread reader_count is still more than 0, it will be
increased because it means this thread is still polling in somewhere.

Signed-off-by: Boram Park <boram1288.park at samsung.com>
---
 src/wayland-client.c | 79 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 78 insertions(+), 1 deletion(-)

diff --git a/src/wayland-client.c b/src/wayland-client.c
index 33033e7..3b80dfa 100644
--- a/src/wayland-client.c
+++ b/src/wayland-client.c
@@ -77,6 +77,10 @@ struct wl_event_queue {
 	struct wl_display *display;
 };
 
+struct wl_thread_data {
+	int reader_count_in_thread;
+};
+
 struct wl_display {
 	struct wl_proxy proxy;
 	struct wl_connection *connection;
@@ -107,6 +111,8 @@ struct wl_display {
 	int reader_count;
 	uint32_t read_serial;
 	pthread_cond_t reader_cond;
+
+	pthread_key_t thread_data_key;
 };
 
 /** \endcond */
@@ -785,6 +791,31 @@ wl_proxy_marshal_array(struct wl_proxy *proxy, uint32_t opcode,
 }
 
 static void
+destroy_thread_data(void *data)
+{
+	struct wl_thread_data *thread_data = data;
+
+	free(thread_data);
+}
+
+static struct wl_thread_data*
+get_thread_data(struct wl_display *display)
+{
+	struct wl_thread_data *thread_data;
+
+	thread_data = pthread_getspecific(display->thread_data_key);
+	if (!thread_data) {
+		thread_data = zalloc(sizeof *thread_data);
+		if (!thread_data)
+			return NULL;
+		thread_data->reader_count_in_thread = 0;
+		pthread_setspecific(display->thread_data_key, thread_data);
+	}
+
+	return thread_data;
+}
+
+static void
 display_handle_error(void *data,
 		     struct wl_display *display, void *object,
 		     uint32_t code, const char *message)
@@ -905,6 +936,7 @@ WL_EXPORT struct wl_display *
 wl_display_connect_to_fd(int fd)
 {
 	struct wl_display *display;
+	struct wl_thread_data *thread_data;
 	const char *debug;
 
 	debug = getenv("WAYLAND_DEBUG");
@@ -960,6 +992,13 @@ wl_display_connect_to_fd(int fd)
 	if (display->connection == NULL)
 		goto err_connection;
 
+	if (pthread_key_create(&display->thread_data_key, destroy_thread_data) < 0)
+		goto err_connection;
+
+	thread_data = get_thread_data(display);
+	if (!thread_data)
+		goto err_connection;
+
 	return display;
 
  err_connection:
@@ -1023,6 +1062,12 @@ wl_display_connect(const char *name)
 WL_EXPORT void
 wl_display_disconnect(struct wl_display *display)
 {
+	struct wl_thread_data *thread_data;
+
+	thread_data = get_thread_data(display);
+	free(thread_data);
+	pthread_key_delete(display->thread_data_key);
+
 	wl_connection_destroy(display->connection);
 	wl_map_release(&display->objects);
 	wl_event_queue_release(&display->default_queue);
@@ -1297,9 +1342,14 @@ dispatch_event(struct wl_display *display, struct wl_event_queue *queue)
 static int
 read_events(struct wl_display *display)
 {
+	struct wl_thread_data *thread_data;
 	int total, rem, size;
 	uint32_t serial;
 
+	thread_data = get_thread_data(display);
+	assert(thread_data);
+
+	thread_data->reader_count_in_thread--;
 	display->reader_count--;
 	if (display->reader_count == 0) {
 		total = wl_connection_read(display->connection);
@@ -1309,6 +1359,9 @@ read_events(struct wl_display *display)
 				 * the reader_count dropped to 0 */
 				display_wakeup_threads(display);
 
+				if (thread_data->reader_count_in_thread > 0)
+					display->reader_count++;
+
 				return 0;
 			}
 
@@ -1346,15 +1399,30 @@ read_events(struct wl_display *display)
 		}
 	}
 
+	/* If reader_count_in_thread > 0, it means that this thread is still polling
+	 * in somewhere. So inclease +1 for it.
+	 */
+	if (thread_data->reader_count_in_thread > 0)
+		display->reader_count++;
+
 	return 0;
 }
 
 static void
 cancel_read(struct wl_display *display)
 {
+	struct wl_thread_data *thread_data;
+
+	thread_data = get_thread_data(display);
+	assert(thread_data);
+
+	thread_data->reader_count_in_thread--;
 	display->reader_count--;
 	if (display->reader_count == 0)
 		display_wakeup_threads(display);
+
+	if (thread_data->reader_count_in_thread > 0)
+		display->reader_count++;
 }
 
 /** Read events from display file descriptor
@@ -1511,7 +1579,16 @@ wl_display_prepare_read_queue(struct wl_display *display,
 		errno = EAGAIN;
 		ret = -1;
 	} else {
-		display->reader_count++;
+		struct wl_thread_data *thread_data;
+
+		thread_data = get_thread_data(display);
+		assert(thread_data);
+
+		/* increase +1 per thread */
+		if (thread_data->reader_count_in_thread == 0)
+			display->reader_count++;
+
+		thread_data->reader_count_in_thread++;
 		ret = 0;
 	}
 
-- 
1.9.1



More information about the wayland-devel mailing list