<div dir="ltr">This patch is now obsolete, see <a href="http://lists.freedesktop.org/archives/wayland-devel/2014-August/016966.html">http://lists.freedesktop.org/archives/wayland-devel/2014-August/016966.html</a> </div> <div class="gmail_extra"> <div class="gmail_quote">On 28 August 2014 12:20, Marek Chalupa <<a href="mailto:mchqwerty@gmail.com" target="_blank">mchqwerty@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"> <div dir="ltr"><div><div><div>Hmm, thinking about that... Maybe this is not the right solution. Programmer should probably do something like: </div>do { ret = wl_display_read_events(display); } while (ret == 0 && errno == EAGAIN); </div>to be sure he read the events (or got an error). This patch would force him to re-run wl_display_prepare_read again </div>in all threads. </div><div class="HOEnZb"><div class="h5"><div class="gmail_extra"> <div class="gmail_quote">On 28 August 2014 11:32, Marek Chalupa <<a href="mailto:mchqwerty@gmail.com" target="_blank">mchqwerty@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">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 <<a href="mailto:mchqwerty@gmail.com" target="_blank">mchqwerty@gmail.com</a>> --- 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 </blockquote></div> </div> </div></div></blockquote></div> </div>