[Xcb] [PATCH libxcb 2/3] Prevent event waiters from blocking reply waiters.

[Rami Ylimäki]

Concurrent calls to xcb_wait_for_event and xcb_wait_for_reply may make
xcb_wait_for_reply block needlessly until an event arrives.

Signed-off-by: Rami Ylimäki <rami.ylimaki at vincit.fi>
---
 src/xcb_conn.c |   12 ++++++++++--
 src/xcb_in.c   |   56 ++++++++++++++++++++++++++++++++++++++++++++++++++------
 src/xcbint.h   |    1 +
 3 files changed, 61 insertions(+), 8 deletions(-)

diff --git a/src/xcb_conn.c b/src/xcb_conn.c
index 1d37614..e99234c 100644
--- a/src/xcb_conn.c
+++ b/src/xcb_conn.c
@@ -269,6 +269,15 @@ void _xcb_conn_shutdown(xcb_connection_t *c)
     c->has_error = 1;
 }
 
+/**
+ * Check if someone is already reading from a connection or is just
+ * about to start reading.
+ */
+static int permit_reading(xcb_connection_t *c)
+{
+    return !c->in.reading && !c->in.prioritized_readers;
+}
+
 int _xcb_conn_wait(xcb_connection_t *c, pthread_cond_t *cond, struct iovec **vector, int *count)
 {
     int ret;
@@ -279,7 +288,7 @@ int _xcb_conn_wait(xcb_connection_t *c, pthread_cond_t *cond, struct iovec **vec
 #endif
 
     /* If the thing I should be doing is already being done, wait for it. */
-    if(count ? c->out.writing : c->in.reading)
+    if(count ? c->out.writing : !permit_reading(c))
     {
         pthread_cond_wait(cond, &c->iolock);
         return 1;
@@ -294,7 +303,6 @@ int _xcb_conn_wait(xcb_connection_t *c, pthread_cond_t *cond, struct iovec **vec
     FD_SET(c->fd, &rfds);
 #endif
     ++c->in.reading;
-
 #if USE_POLL
     if(count)
     {
diff --git a/src/xcb_in.c b/src/xcb_in.c
index 8cfc5c9..7b83da6 100644
--- a/src/xcb_in.c
+++ b/src/xcb_in.c
@@ -358,6 +358,21 @@ static int poll_for_reply(xcb_connection_t *c, unsigned int request, void **repl
 
 /* Public interface */
 
+/**
+ * If an event reader gave priority to reply reader, the reply reader
+ * can use this function to give the event reader a permission to
+ * continue.
+ */
+static void permit_event_reader(xcb_connection_t *c)
+{
+    if (c->in.prioritized_readers)
+    {
+        int pthreadret = pthread_cond_signal(&c->in.event_cond);
+        c->in.prioritized_readers = 0;
+        assert(pthreadret == 0);
+    }
+}
+
 void *xcb_wait_for_reply(xcb_connection_t *c, unsigned int request, xcb_generic_error_t **e)
 {
     uint64_t widened_request;
@@ -379,6 +394,7 @@ void *xcb_wait_for_reply(xcb_connection_t *c, unsigned int request, xcb_generic_
         pthread_cond_t cond = PTHREAD_COND_INITIALIZER;
         reader_list reader;
         reader_list **prev_reader;
+        int status = 1;
 
         for(prev_reader = &c->in.readers; 
 	    *prev_reader && 
@@ -392,9 +408,13 @@ void *xcb_wait_for_reply(xcb_connection_t *c, unsigned int request, xcb_generic_
         reader.next = *prev_reader;
         *prev_reader = &reader;
 
-        while(!poll_for_reply(c, request, &ret, e))
-            if(!_xcb_conn_wait(c, &cond, 0, 0))
-                break;
+        /* Wait for response from the connection. If an event reader
+         * has given priority to us, we must let it continue. */
+        while(status && !poll_for_reply(c, request, &ret, e))
+        {
+            status = _xcb_conn_wait(c, &cond, 0, 0);
+            permit_event_reader(c);
+        }
 
         for(prev_reader = &c->in.readers;
 	    *prev_reader && 
@@ -542,16 +562,39 @@ int xcb_poll_for_reply(xcb_connection_t *c, unsigned int request, void **reply,
     return ret;
 }
 
+/**
+ * If an event reader received a reply instead of an event, it can
+ * wake up next reply reader to check that reply with this
+ * function. Otherwise an event reader may cause the reply reader to
+ * block forever if events don't arrive.
+ */
+static void permit_reply_reader(xcb_connection_t *c)
+{
+    if (c->in.readers)
+    {
+        int pthreadret = pthread_cond_signal(c->in.readers->data);
+        c->in.prioritized_readers = 1;
+        assert(pthreadret == 0);
+    }
+}
+
 xcb_generic_event_t *xcb_wait_for_event(xcb_connection_t *c)
 {
     xcb_generic_event_t *ret;
     if(c->has_error)
         return 0;
     pthread_mutex_lock(&c->iolock);
+
     /* get_event returns 0 on empty list. */
-    while(!(ret = get_event(c)))
-        if(!_xcb_conn_wait(c, &c->in.event_cond, 0, 0))
-            break;
+    ret = get_event(c);
+    /* Wait for response from the connection. If the response wasn't
+     * an event, let a reply reader get it. Otherwise the reply reader
+     * could block until an event arrives. */
+    while(!ret && _xcb_conn_wait(c, &c->in.event_cond, 0, 0))
+    {
+        if (!(ret = get_event(c)))
+            permit_reply_reader(c);
+    }
 
     wake_up_next_reader(c);
     pthread_mutex_unlock(&c->iolock);
@@ -613,6 +656,7 @@ int _xcb_in_init(_xcb_in *in)
     if(pthread_cond_init(&in->event_cond, 0))
         return 0;
     in->reading = 0;
+    in->prioritized_readers = 0;
 
     in->queue_len = 0;
 
diff --git a/src/xcbint.h b/src/xcbint.h
index 154cca0..122c8c9 100644
--- a/src/xcbint.h
+++ b/src/xcbint.h
@@ -115,6 +115,7 @@ int _xcb_out_flush_to(xcb_connection_t *c, uint64_t request);
 typedef struct _xcb_in {
     pthread_cond_t event_cond;
     int reading;
+    int prioritized_readers;
 
     char queue[4096];
     int queue_len;
-- 
1.6.3.3