Attachment 50562 Details for Bug 40372 – Proposed patch

[patch] Proposed patch

0001-Fix-a-dead-lock-due-to-xcb_poll_for_reply.patch (text/plain), 2.40 KB, created by Uli Schlachter on 2011-08-25 05:28:45 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Uli Schlachter

Created: 2011-08-25 05:28:45 UTC

Size: 2.40 KB

patch

obsolete

>From 5f635ec1e3ba70de2e8063f6f1a6163157220033 Mon Sep 17 00:00:00 2001
>From: Uli Schlachter <psychon@znc.in>
>Date: Thu, 25 Aug 2011 14:18:16 +0200
>Subject: [PATCH] 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.
>
>Signed-off-by: Uli Schlachter <psychon@znc.in>
>---
> src/xcb_in.c |    2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
>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);
>     }
>-- 
>1.7.5.4
>

Actions: View | Splinter Review

Attachments on bug 40372: 50561 | 50562

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.