Fix a dead-lock due to xcb_poll_for_reply
authorUli Schlachter <psychon@znc.in>
Thu, 25 Aug 2011 12:18:16 +0000 (14:18 +0200)
committerPeter Harris <pharris@opentext.com>
Fri, 2 Sep 2011 17:01:34 +0000 (13:01 -0400)
Imagine two threads:

Thread#1: for(;;) { xcb_get_input_focus_reply(c, xcb_get_input_focus(c), 0); }

Thread#2: for(;;) { xcb_poll_for_event(c); }

Since xcb_poll_for_event() calls _xcb_in_read() directly without synchronizing
with any other readers, this causes two threads to end up calling recv() at the
same time. We now have a race because any of these two threads could get read
the GetInputFocus reply.

If thread#2 reads this reply, it will be put in the appropriate queue and
thread#1 will still be stuck in recv(), although its reply was already received.
If no other reply or event causes this thread to wake up, the process deadlocks.

To fix this, we have to make sure that there is only ever one thread reading
from the connection. The obvious solution is to check in poll_for_next_event()
if another thread is already reading (in which case c->in.reading != 0) and not
to read from the wire in this case.

This solution is actually correct if we assume that the other thread is blocked
in poll() which means there isn't any data which can be read. Since we already
checked that there is no event in the queue this means that
poll_for_next_event() didn't find any event to return.

There might be a small race here where the other thread already determined that
there is data to read, but it still has to wait for c->iolock. However, this
means that the next poll_for_next_event() will be able to read the event, so
this shouldn't cause any problems.

Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=40372

Signed-off-by: Uli Schlachter <psychon@znc.in>
Signed-off-by: Peter Harris <pharris@opentext.com>
src/xcb_in.c

index 4acc3a2..e075a40 100644 (file)
@@ -535,7 +535,7 @@ static xcb_generic_event_t *poll_for_next_event(xcb_connection_t *c, int queued)
         pthread_mutex_lock(&c->iolock);
         /* FIXME: follow X meets Z architecture changes. */
         ret = get_event(c);
-        if(!ret && !queued && _xcb_in_read(c)) /* _xcb_in_read shuts down the connection on error */
+        if(!ret && !queued && c->in.reading == 0 && _xcb_in_read(c)) /* _xcb_in_read shuts down the connection on error */
             ret = get_event(c);
         pthread_mutex_unlock(&c->iolock);
     }