[Xcb] [PATCH] Fix a thread hang with xcb_wait_for_special_event()

[Michel Dänzer]

On 12.06.2015 22:21, Uli Schlachter wrote:
> 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 at znc.in>
> ---
> Michel, could you try if the test case at [0] also hangs for you and if this
> patch fixes both your original problem and this test? Thanks.

The answer to all of these questions is yes. :)


> +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;
> +}

AFAICT this inserts the new special_list entry at the beginning of the
linked list, so the thread corresponding to the last inserted entry will
be woken up first, right? It might be fairer to make this a FIFO instead
of a LIFO.


Other than that, it looks good to me.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer