[Xcb-commit] src

Peter Harris peterh at kemper.freedesktop.org
Fri Sep 2 10:03:58 PDT 2011


 src/xcb_in.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

New commits:
commit 5ceeaaa4294201b3f613c07f9ec610c0e5f673c7
Author: Uli Schlachter <psychon at znc.in>
Date:   Thu Aug 25 14:18:16 2011 +0200

    Fix a dead-lock due to xcb_poll_for_reply
    
    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 at znc.in>
    Signed-off-by: Peter Harris <pharris at opentext.com>

diff --git a/src/xcb_in.c b/src/xcb_in.c
index 4acc3a2..e075a40 100644
--- a/src/xcb_in.c
+++ b/src/xcb_in.c
@@ -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);
     }


More information about the xcb-commit mailing list