[Xcb] [PATCH xcb] don't flag extra reply in xcb_take_socket

[Uli Schlachter]

Thanks. I took the commit message from your original mail, added it to
this patch and pushed the result.

Cheers,
Uli

P.S.: And now I'll wait for someone to ask for an 1.13.1 release. Plus
someone who volunteers to do that release.

On 20.08.2018 21:06, Erik Kurzinger wrote:
> Hi Uli,
> 
> Thanks for taking a look! I tried modifying the 'else' case in _xcb_in_replies_done
> as you suggested, I agree that it's a bit cleaner than what I had. 
> 
> It still appears to fix the hang in the example program as well as the KWin crash
> I had mentioned so everything looks good. I can't think of a better name than 
> 'prev_next' either - it'll have to do :)
> 
> Cheers,
> Erik
> 
> ---
>  src/xcb_in.c  | 16 ++++++++++++++--
>  src/xcb_out.c | 10 ++++++++--
>  2 files changed, 22 insertions(+), 4 deletions(-)
> 
> diff --git a/src/xcb_in.c b/src/xcb_in.c
> index 73209e0..58fe896 100644
> --- a/src/xcb_in.c
> +++ b/src/xcb_in.c
> @@ -958,8 +958,20 @@ void _xcb_in_replies_done(xcb_connection_t *c)
>          pend = container_of(c->in.pending_replies_tail, struct pending_reply, next);
>          if(pend->workaround == WORKAROUND_EXTERNAL_SOCKET_OWNER)
>          {
> -            pend->last_request = c->out.request;
> -            pend->workaround = WORKAROUND_NONE;
> +            if (XCB_SEQUENCE_COMPARE(pend->first_request, <=, c->out.request)) {
> +                pend->last_request = c->out.request;
> +                pend->workaround = WORKAROUND_NONE;
> +            } else {
> +                /* The socket was taken, but no requests were actually sent
> +                 * so just discard the pending_reply that was created.
> +                 */
> +                struct pending_reply **prev_next = &c->in.pending_replies;
> +                while (*prev_next != pend)
> +                    prev_next = &(*prev_next)->next;
> +                *prev_next = NULL;
> +                c->in.pending_replies_tail = prev_next;
> +                free(pend);
> +            }
>          }
>      }
>  }
> 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;
>      }
> 


-- 
If you have to type the letters "A-E-S" into your source code, you're
doing it wrong.