[Xcb] [PATCH] Terminate the connection if the fd was closed on us.

[Jamey Sharp]

Reviewed-by: Jamey Sharp <jamey at minilop.net>

I always prefer readability, especially if the compiler will generate
the same code either way.

I might have chosen to make the block that calls poll set "ret" like
select does, to reduce the piling of conditionals, but I hardly care.

When this patch is committed, I believe we have a couple of important
correctness fixes that haven't been released yet. Perhaps it's time?

Jamey

On 7/5/10, Peter Hutterer <peter.hutterer at who-t.net> wrote:
> If a client calls close(2) on the connection's file descriptor and then
> XFlush(3), libxcb causes a hang in the client.
>
> XFlush(3) eventually calls _xcb_out_send() with has the following loop:
>     while(ret && *count)
>         ret = _xcb_conn_wait(c, &c->out.cond, vector, count);
>
> _xcb_conn_wait(), if built with USE_POLL gets the POLLNVAL error. It only
> checks for POLLIN and POLLOUT though, ignoring the error.  Return value is
> 1, count is unmodified, leaving us with an endless loop and the a client
> hang.
>
> This patch adds a check for POLLNVAL in the polling code. If POLLNVAL is
> detected, _xcb_conn_shutdown() is invoked an and error is returned. This
> matches the behaviour if select(2) is used instead of poll(2). select(2)
> returns -1 and EBADF for an already closed file descriptor.
>
> Fixes XTS testcase Xlib3/XConnectionNumber.
> http://cgit.freedesktop.org/xorg/test/xts/tree/xts5/Xlib3/XConnectionNumber.m
>
> Signed-off-by: Peter Hutterer <peter.hutterer at who-t.net>
> ---
> I opted for the double (ret < 0) for readabilty, if a construct like
>
>    if (ret < 0
> #if USE_POLL
>         || ((fd.revents & POLLNVAL) == POLLNVAL))
> #endif
>     )
>
> is preferred, I can fix up the patch.
>
>  src/xcb_conn.c |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/src/xcb_conn.c b/src/xcb_conn.c
> index 251d62e..a96186b 100644
> --- a/src/xcb_conn.c
> +++ b/src/xcb_conn.c
> @@ -308,7 +308,11 @@ int _xcb_conn_wait(xcb_connection_t *c, pthread_cond_t
> *cond, struct iovec **vec
>  	ret = select(c->fd + 1, &rfds, &wfds, 0, 0);
>  #endif
>      } while (ret == -1 && errno == EINTR);
> +#if USE_POLL
> +    if (ret < 0 || ((fd.revents & POLLNVAL) == POLLNVAL))
> +#else
>      if (ret < 0)
> +#endif
>      {
>          _xcb_conn_shutdown(c);
>  	ret = 0;
> --
> 1.7.1