Attachment 116463 Details for Bug 84252 – Make _xcb_in_wake_up_next_reader() wake up threads in xcb_wait_for_special_event()

[patch] Make _xcb_in_wake_up_next_reader() wake up threads in xcb_wait_for_special_event()

0001-Fix-a-thread-hang-with-xcb_wait_for_special_event.patch (text/plain), 4.35 KB, created by Uli Schlachter on 2015-06-12 13:24:42 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Uli Schlachter

Created: 2015-06-12 13:24:42 UTC

Size: 4.35 KB

patch

obsolete

>From e37e42062fc9f320b0ce4cd0f654fe118b5acb6b Mon Sep 17 00:00:00 2001
>From: Uli Schlachter <psychon@znc.in>
>Date: Fri, 12 Jun 2015 15:13:05 +0200
>Subject: [PATCH] Fix a thread hang with xcb_wait_for_special_event()
>
>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>
>---
> src/xcb_in.c | 32 ++++++++++++++++++++++++++++++++
> src/xcbint.h |  1 +
> 2 files changed, 33 insertions(+)
>
>diff --git a/src/xcb_in.c b/src/xcb_in.c
>index 322bed8..bab4bc7 100644
>--- a/src/xcb_in.c
>+++ b/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;
>@@ -750,17 +775,22 @@ 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;
>@@ -889,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);
>diff --git a/src/xcbint.h b/src/xcbint.h
>index f89deba..acce646 100644
>--- a/src/xcbint.h
>+++ b/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;
>-- 
>2.1.4
>

Actions: View | Splinter Review

Attachments on bug 84252: 106805 | 115165 | 115200 | 115827 | 115887 | 116462 | 116463 | 121283

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.