[prev in list] [next in list] [prev in thread] [next in thread] 

List:       freedesktop-xorg-devel
Subject:    Re: [Xcb] [PATCH xcb] don't flag extra reply in xcb_take_socket
From:       Uli Schlachter <psychon () znc ! in>
Date:       2018-08-18 7:47:59
Message-ID: 1b415f44-4f28-6fa0-f4f0-365391d5aed4 () znc ! in
[Download RAW message or body]

On 14.08.2018 14:46, Julien Cristau wrote:
> +xcb@

Thanks, Julien.

And sorry for this mail getting stuck in xorg-devel's moderation queue,
I'll remove that CC if I reply again.

> On 08/09/2018 12:20 AM, Erik Kurzinger wrote:
>> If any flags are specified in a call to xcb_take_socket,
>> they should only be applied to replies for requests sent
>> after that function returns (and until the socket is
>> re-acquired by XCB).
>>
>> Previously, they would also be incorrectly applied to the
>> reply for the last request sent before the socket was taken.
>> For instance, in this example program the reply for the
>> GetInputFocus request gets discarded, even though it was
>> sent before the socket was taken. This results in the
>> call to retrieve the reply hanging indefinitely.

Thanks for figuring this out. Still, I'm slightly confused about this. I
added another GetInputFocus request after the xcb_take_socket(). If I
get the reply for this second request first, everything works fine. If I
get the reply for this second request second, getting the first reply
still hangs. (See attached file)

I fail to understand this behaviour. Since pending replies are applied
when receiving responses from the server, shouldn't the order that I
actually fetch the replies from XCB make no difference?

(Also, I patched my local xcb with just your change to
xcb_take_socket(), expecting this to cause the first request after
taking the socket to be discarded, but this did not happen either)

I feel like I am understanding less than before I started to figure this
out...

Cheers,
Uli

[...]
>> diff --git a/src/xcb_out.c b/src/xcb_out.c
>> index 3601a5f..c9593e5 100644
>> --- a/src/xcb_out.c
>> +++ b/src/xcb_out.c
>> @@ -387,8 +387,14 @@ int xcb_take_socket(xcb_connection_t *c, void (*return_socket)(void *closure), v
>>      {
>>          c->out.return_socket = return_socket;
>>          c->out.socket_closure = closure;
>> -        if(flags)
>> -            _xcb_in_expect_reply(c, c->out.request, WORKAROUND_EXTERNAL_SOCKET_OWNER, flags);
>> +        if(flags) {
>> +            /* c->out.request + 1 will be the first request sent by the external
>> +             * socket owner. If the socket is returned before this request is sent
>> +             * it will be detected in _xcb_in_replies_done and this pending_reply
>> +             * will be discarded.
>> +             */
>> +            _xcb_in_expect_reply(c, c->out.request + 1, WORKAROUND_EXTERNAL_SOCKET_OWNER, flags);
>> +        }
>>          assert(c->out.request == c->out.request_written);
>>          *sent = c->out.request;
>>      }
>>
-- 
- Buck, when, exactly, did you lose your mind?
- Three months ago. I woke up one morning married to a pineapple.
  An ugly pineapple... But I loved her.

["test.c" (text/x-csrc)]

#include <stdio.h>
#include <xcb/xcb.h>
#include <xcb/xcbext.h>

static void return_socket(void *closure) {
	(void) closure;
}

int main(void)
{
    xcb_connection_t *c = xcb_connect(NULL, NULL);

    xcb_get_input_focus_cookie_t cookie1 = xcb_get_input_focus_unchecked(c);

    uint64_t seq;
    xcb_take_socket(c, return_socket, NULL, XCB_REQUEST_DISCARD_REPLY, &seq);

    xcb_get_input_focus_cookie_t cookie2 = xcb_get_input_focus_unchecked(c);

    xcb_generic_error_t *err;
#if 0
    // This version does not hang
    puts("before 2");
    xcb_get_input_focus_reply(c, cookie2, &err);
    puts("before 1");
    xcb_get_input_focus_reply(c, cookie1, &err);
#else
    // This version hangs
    puts("before 1");
    xcb_get_input_focus_reply(c, cookie1, &err);
    puts("before 2");
    xcb_get_input_focus_reply(c, cookie2, &err);
#endif
    puts("done");
}

[Attachment #4 (text/plain)]

_______________________________________________
xorg-devel@lists.x.org: X.Org development
Archives: http://lists.x.org/archives/xorg-devel
Info: https://lists.x.org/mailman/listinfo/xorg-devel

[prev in list] [next in list] [prev in thread] [next in thread]