Release libxcb 1.11.1

Consider the following:

- Two threads are calling xcb_wait_for_special_event() and xcb_wait_for_reply()
  concurrently.
- The thread doing xcb_wait_for_reply() wins the race and poll()s the socket for
  readability.
- The other thread will be put to sleep on the special_event_cond of the special
  event (this is done in _xcb_conn_wait() via the argument
  xcb_wait_for_special_event() gives it).
- The first thread gets its reply, but does not yet receive any special event.

In this case, the first thread will return to its caller. On its way out, it
will call _xcb_in_wake_up_next_reader(), but that function cannot wake up
anything since so far it did not handle xcb_wait_for_special_event().

Thus, the first thread stays blocked on the condition variable and no thread
tries to read from the socket.

A test case demonstrating this problem is available at the bug report.

Fix this similar to how we handle this with xcb_wait_for_reply():

The function wait_for_reply() adds an entry into a linked list of threads that
wait for a reply. Via this list, _xcb_in_wake_up_next_reader() can wake up this
thread so that it can call _xcb_conn_wait() again and then poll()s the socket.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=84252
Signed-off-by: Uli Schlachter <psychon@znc.in>
Tested-by: Michel Dänzer <michel.daenzer@amd.com>

NEWS

@@ -1,3 +1,8 @@
+Release 1.11.1 (2015-09-06)
+===========================
+* Expose 64-bit sequence numbers for XLib
+* Fix some hangs related to xcb_wait_for_special_event()
+
 Release 1.11 (2014-08-01)
 =========================
 * Force structures with 64-bit fields to be packed

configure.ac

@@ -2,7 +2,7 @@ dnl Process this file with autoconf to produce a configure script.
 
 # Initialize Autoconf
 AC_PREREQ([2.60])
-AC_INIT([libxcb],[1.11],
+AC_INIT([libxcb],[1.11.1],
         [https://bugs.freedesktop.org/enter_bug.cgi?product=xcb],
         [libxcb])
 AC_CONFIG_AUX_DIR([build-aux])

src/xcb.h

@@ -378,6 +378,26 @@ xcb_generic_error_t *xcb_request_check(xcb_connection_t *c, xcb_void_cookie_t co
  */
 void xcb_discard_reply(xcb_connection_t *c, unsigned int sequence);
 
+/**
+ * @brief Discards the reply for a request, given by a 64bit sequence number
+ * @param c: The connection to the X server.
+ * @param sequence: 64-bit sequence number as returned by xcb_send_request64().
+ *
+ * Discards the reply for a request. Additionally, any error generated
+ * by the request is also discarded (unless it was an _unchecked request
+ * and the error has already arrived).
+ *
+ * This function will not block even if the reply is not yet available.
+ *
+ * Note that the sequence really does have to come from xcb_send_request64();
+ * the cookie sequence number is defined as "unsigned" int and therefore
+ * not 64-bit on all platforms.
+ * This function is not designed to operate on socket-handoff replies.
+ *
+ * Unlike its xcb_discard_reply() counterpart, the given sequence number is not
+ * automatically "widened" to 64-bit.
+ */
+void xcb_discard_reply64(xcb_connection_t *c, uint64_t sequence);
 
 /* xcb_ext.c */
 

src/xcb_in.c

@@ -97,6 +97,11 @@ typedef struct reader_list {
     struct reader_list *next;
 } reader_list;
 
+typedef struct special_list {
+    xcb_special_event_t *se;
+    struct special_list *next;
+} special_list;
+
 static void remove_finished_readers(reader_list **prev_reader, uint64_t completed)
 {
     while(*prev_reader && XCB_SEQUENCE_COMPARE((*prev_reader)->request, <=, completed))
@@ -475,6 +480,26 @@ static void remove_reader(reader_list **prev_reader, reader_list *reader)
         }
 }
 
+static void insert_special(special_list **prev_special, special_list *special, xcb_special_event_t *se)
+{
+    special->se = se;
+    special->next = *prev_special;
+    *prev_special = special;
+}
+
+static void remove_special(special_list **prev_special, special_list *special)
+{
+    while(*prev_special)
+    {
+        if(*prev_special == special)
+        {
+            *prev_special = (*prev_special)->next;
+            break;
+        }
+        prev_special = &(*prev_special)->next;
+    }
+}
+
 static void *wait_for_reply(xcb_connection_t *c, uint64_t request, xcb_generic_error_t **e)
 {
     void *ret = 0;
@@ -523,6 +548,20 @@ void *xcb_wait_for_reply(xcb_connection_t *c, unsigned int request, xcb_generic_
     return ret;
 }
 
+void *xcb_wait_for_reply64(xcb_connection_t *c, uint64_t request, xcb_generic_error_t **e)
+{
+    void *ret;
+    if(e)
+        *e = 0;
+    if(c->has_error)
+        return 0;
+
+    pthread_mutex_lock(&c->iolock);
+    ret = wait_for_reply(c, request, e);
+    pthread_mutex_unlock(&c->iolock);
+    return ret;
+}
+
 int *xcb_get_reply_fds(xcb_connection_t *c, void *reply, size_t reply_size)
 {
     return (int *) (&((char *) reply)[reply_size]);
@@ -595,6 +634,20 @@ void xcb_discard_reply(xcb_connection_t *c, unsigned int sequence)
     pthread_mutex_unlock(&c->iolock);
 }
 
+void xcb_discard_reply64(xcb_connection_t *c, uint64_t sequence)
+{
+    if(c->has_error)
+        return;
+
+    /* If an error occurred when issuing the request, fail immediately. */
+    if(!sequence)
+        return;
+
+    pthread_mutex_lock(&c->iolock);
+    discard_reply(c, sequence);
+    pthread_mutex_unlock(&c->iolock);
+}
+
 int xcb_poll_for_reply(xcb_connection_t *c, unsigned int request, void **reply, xcb_generic_error_t **error)
 {
     int ret;
@@ -612,6 +665,23 @@ int xcb_poll_for_reply(xcb_connection_t *c, unsigned int request, void **reply,
     return ret;
 }
 
+int xcb_poll_for_reply64(xcb_connection_t *c, uint64_t request, void **reply, xcb_generic_error_t **error)
+{
+    int ret;
+    if(c->has_error)
+    {
+        *reply = 0;
+        if(error)
+            *error = 0;
+        return 1; /* would not block */
+    }
+    assert(reply != 0);
+    pthread_mutex_lock(&c->iolock);
+    ret = poll_for_reply(c, request, reply, error);
+    pthread_mutex_unlock(&c->iolock);
+    return ret;
+}
+
 xcb_generic_event_t *xcb_wait_for_event(xcb_connection_t *c)
 {
     xcb_generic_event_t *ret;
@@ -705,17 +775,23 @@ xcb_generic_event_t *xcb_poll_for_special_event(xcb_connection_t *c,
 xcb_generic_event_t *xcb_wait_for_special_event(xcb_connection_t *c,
                                                 xcb_special_event_t *se)
 {
+    special_list special;
     xcb_generic_event_t *event;
 
     if(c->has_error)
         return 0;
     pthread_mutex_lock(&c->iolock);
 
+    insert_special(&c->in.special_waiters, &special, se);
+
     /* get_special_event returns 0 on empty list. */
     while(!(event = get_special_event(c, se)))
         if(!_xcb_conn_wait(c, &se->special_event_cond, 0, 0))
             break;
 
+    remove_special(&c->in.special_waiters, &special);
+
+    _xcb_in_wake_up_next_reader(c);
     pthread_mutex_unlock(&c->iolock);
     return event;
 }
@@ -843,6 +919,8 @@ void _xcb_in_wake_up_next_reader(xcb_connection_t *c)
     int pthreadret;
     if(c->in.readers)
         pthreadret = pthread_cond_signal(c->in.readers->data);
+    else if(c->in.special_waiters)
+        pthreadret = pthread_cond_signal(&c->in.special_waiters->se->special_event_cond);
     else
         pthreadret = pthread_cond_signal(&c->in.event_cond);
     assert(pthreadret == 0);

src/xcb_out.c

@@ -177,7 +177,7 @@ uint32_t xcb_get_maximum_request_length(xcb_connection_t *c)
     return c->out.maximum_request_length.value;
 }
 
-unsigned int xcb_send_request(xcb_connection_t *c, int flags, struct iovec *vector, const xcb_protocol_request_t *req)
+uint64_t xcb_send_request64(xcb_connection_t *c, int flags, struct iovec *vector, const xcb_protocol_request_t *req)
 {
     uint64_t request;
     uint32_t prefix[2];
@@ -286,6 +286,12 @@ unsigned int xcb_send_request(xcb_connection_t *c, int flags, struct iovec *vect
     return request;
 }
 
+/* request number are actually uint64_t internally but keep API compat with unsigned int */
+unsigned int xcb_send_request(xcb_connection_t *c, int flags, struct iovec *vector, const xcb_protocol_request_t *req)
+{
+    return xcb_send_request64(c, flags, vector, req);
+}
+
 void
 xcb_send_fd(xcb_connection_t *c, int fd)
 {

src/xcbext.h

@@ -82,6 +82,30 @@ enum xcb_send_request_flags_t {
  */
 unsigned int xcb_send_request(xcb_connection_t *c, int flags, struct iovec *vector, const xcb_protocol_request_t *request);
 
+/**
+ * @brief Send a request to the server, with 64-bit sequence number returned.
+ * @param c: The connection to the X server.
+ * @param flags: A combination of flags from the xcb_send_request_flags_t enumeration.
+ * @param vector: Data to send; must have two iovecs before start for internal use.
+ * @param request: Information about the request to be sent.
+ * @return The request's sequence number on success, 0 otherwise.
+ *
+ * This function sends a new request to the X server. The data of the request is
+ * given as an array of @c iovecs in the @p vector argument. The length of that
+ * array and the neccessary management information are given in the @p request
+ * argument.
+ *
+ * When this function returns, the request might or might not be sent already.
+ * Use xcb_flush() to make sure that it really was sent.
+ *
+ * Please note that this function is not the prefered way for sending requests.
+ * It's better to use the generated wrapper functions.
+ *
+ * Please note that xcb might use index -1 and -2 of the @p vector array internally,
+ * so they must be valid!
+ */
+uint64_t xcb_send_request64(xcb_connection_t *c, int flags, struct iovec *vector, const xcb_protocol_request_t *request);
+
 /**
  * @brief Send a file descriptor to the server in the next call to xcb_send_request.
  * @param c: The connection to the X server.
@@ -161,6 +185,21 @@ int xcb_writev(xcb_connection_t *c, struct iovec *vector, int count, uint64_t re
  */
 void *xcb_wait_for_reply(xcb_connection_t *c, unsigned int request, xcb_generic_error_t **e);
 
+/**
+ * @brief Wait for the reply of a given request, with 64-bit sequence number
+ * @param c: The connection to the X server.
+ * @param request: 64-bit sequence number of the request as returned by xcb_send_request64().
+ * @param e: Location to store errors in, or NULL. Ignored for unchecked requests.
+ *
+ * Returns the reply to the given request or returns null in the event of
+ * errors. Blocks until the reply or error for the request arrives, or an I/O
+ * error occurs.
+ *
+ * Unlike its xcb_wait_for_reply() counterpart, the given sequence number is not
+ * automatically "widened" to 64-bit.
+ */
+void *xcb_wait_for_reply64(xcb_connection_t *c, uint64_t request, xcb_generic_error_t **e);
+
 /**
  * @brief Poll for the reply of a given request.
  * @param c: The connection to the X server.
@@ -173,6 +212,21 @@ void *xcb_wait_for_reply(xcb_connection_t *c, unsigned int request, xcb_generic_
  */
 int xcb_poll_for_reply(xcb_connection_t *c, unsigned int request, void **reply, xcb_generic_error_t **error);
 
+/**
+ * @brief Poll for the reply of a given request, with 64-bit sequence number.
+ * @param c: The connection to the X server.
+ * @param request: 64-bit sequence number of the request as returned by xcb_send_request().
+ * @param reply: Location to store the reply in, must not be NULL.
+ * @param e: Location to store errors in, or NULL. Ignored for unchecked requests.
+ * @return 1 when the reply to the request was returned, else 0.
+ *
+ * Checks if the reply to the given request already received. Does not block.
+ *
+ * Unlike its xcb_poll_for_reply() counterpart, the given sequence number is not
+ * automatically "widened" to 64-bit.
+ */
+int xcb_poll_for_reply64(xcb_connection_t *c, uint64_t request, void **reply, xcb_generic_error_t **error);
+
 /**
  * @brief Don't use this, only needed by the generated code.
  * @param c: The connection to the X server.

src/xcbint.h

@@ -142,6 +142,7 @@ typedef struct _xcb_in {
     struct event_list *events;
     struct event_list **events_tail;
     struct reader_list *readers;
+    struct special_list *special_waiters;
 
     struct pending_reply *pending_replies;
     struct pending_reply **pending_replies_tail;