<div dir="ltr">This patch is now obsolete, see<br><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><br></div>
<div class="gmail_extra"><br><br><div class="gmail_quote">On 28 August 2014 12:20, Marek Chalupa <span dir="ltr"><<a href="mailto:mchqwerty@gmail.com" target="_blank">mchqwerty@gmail.com</a>></span> wrote:<br><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:<br><br></div>do {<br>  ret = wl_display_read_events(display);<br>} while (ret == 0 && errno == EAGAIN);<br>

<br></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<br></div>in all threads.<br></div><div class="HOEnZb"><div class="h5"><div class="gmail_extra">
<br><br><div class="gmail_quote">On 28 August 2014 11:32, Marek Chalupa <span dir="ltr"><<a href="mailto:mchqwerty@gmail.com" target="_blank">mchqwerty@gmail.com</a>></span> wrote:<br>
<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<br>
contains always-repeated pattern: broadcast + increase serial.<br>
<br>
Replace the pattern with this function and call this function also<br>
on return from read_events when wl_connection_read fails with<br>
EAGAIN (this solves the bug from the test from previous commit).<br>
<br>
Now all return paths from read_events are safe and wake up the threads.<br>
There's a path in wl_display_read_events that do not wake up<br>
threads - this is the case when last_error is set. But on this<br>
path are the threads always awake, because every function that can<br>
set last_error sets it via display_fatal_error (which calls<br>
display_wakeup_threads).<br>
<br>
Signed-off-by: Marek Chalupa <<a href="mailto:mchqwerty@gmail.com" target="_blank">mchqwerty@gmail.com</a>><br>
---<br>
 src/wayland-client.c | 42 +++++++++++++++++++++++++++++-------------<br>
 1 file changed, 29 insertions(+), 13 deletions(-)<br>
<br>
diff --git a/src/wayland-client.c b/src/wayland-client.c<br>
index 9159ee0..9574cf7 100644<br>
--- a/src/wayland-client.c<br>
+++ b/src/wayland-client.c<br>
@@ -110,6 +110,27 @@ struct wl_display {<br>
 static int debug_client = 0;<br>
<br>
 /**<br>
+ * This helper function wakes up all threads that are<br>
+ * waiting for display->reader_cond (i. e. when reading is done<br>
+ * canceled, or an error occured)<br>
+ *<br>
+ * NOTE: must be called with display->mutex locked<br>
+ */<br>
+static void<br>
+display_wakeup_threads(struct wl_display *display)<br>
+{<br>
+       pthread_cond_broadcast(&display->reader_cond);<br>
+<br>
+       /* Thread can get sleeping only in read_events() and if we're<br>
+       * waking it up, it means that the read processed or was<br>
+       * canceled, so we must increase the read_serial.<br>
+       * This prevents from indefinite looping in read_events()<br>
+       * (when calling pthread_cond_wait under condition<br>
+       * display->read_serial == serial). */<br>
+       ++display->read_serial;<br>
+}<br>
+<br>
+/**<br>
  * This function is called for local errors (no memory, server hung up)<br>
  *<br>
  * \param display<br>
@@ -128,11 +149,7 @@ display_fatal_error(struct wl_display *display, int error)<br>
<br>
        display->last_error = error;<br>
<br>
-       pthread_cond_broadcast(&display->reader_cond);<br>
-       /* prevent from indefinite looping in read_events()<br>
-       * (when calling pthread_cond_wait under condition<br>
-       * display->read_serial == serial) */<br>
-       ++display->read_serial;<br>
+       display_wakeup_threads(display);<br>
 }<br>
<br>
 /**<br>
@@ -182,7 +199,7 @@ display_protocol_error(struct wl_display *display, uint32_t code,<br>
        display->protocol_error.interface = intf;<br>
<br>
        /*<br>
-        * here it is not necessary to broadcast reader's cond like in<br>
+        * here it is not necessary to wake up threads like in<br>
         * display_fatal_error, because this function is called from<br>
         * an event handler and that means that read_events() is done<br>
         * and woke up all threads. Since wl_display_prepare_read()<br>
@@ -1138,8 +1155,10 @@ read_events(struct wl_display *display)<br>
        if (display->reader_count == 0) {<br>
                total = wl_connection_read(display->connection);<br>
                if (total == -1) {<br>
-                       if (errno == EAGAIN)<br>
+                       if (errno == EAGAIN) {<br>
+                               display_wakeup_threads(display);<br>
                                return 0;<br>
+                       }<br>
<br>
                        display_fatal_error(display, errno);<br>
                        return -1;<br>
@@ -1162,8 +1181,7 @@ read_events(struct wl_display *display)<br>
                        }<br>
                }<br>
<br>
-               display->read_serial++;<br>
-               pthread_cond_broadcast(&display->reader_cond);<br>
+               display_wakeup_threads(display);<br>
        } else {<br>
                serial = display->read_serial;<br>
                while (display->read_serial == serial)<br>
@@ -1348,10 +1366,8 @@ wl_display_cancel_read(struct wl_display *display)<br>
        pthread_mutex_lock(&display->mutex);<br>
<br>
        display->reader_count--;<br>
-       if (display->reader_count == 0) {<br>
-               display->read_serial++;<br>
-               pthread_cond_broadcast(&display->reader_cond);<br>
-       }<br>
+       if (display->reader_count == 0)<br>
+               display_wakeup_threads(display);<br>
<br>
        pthread_mutex_unlock(&display->mutex);<br>
 }<br>
<span><font color="#888888">--<br>
1.9.3<br>
<br>
</font></span></blockquote></div><br></div>
</div></div></blockquote></div><br></div>