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

[Rémi Denis-Courmont]

On Mon, 5 Jul 2010 10:54:26 +1000, Peter Hutterer
<peter.hutterer at who-t.net> wrote:
> If a client calls close(2) on the connection's file descriptor and then

...then the client is broken beyond repair in any case as far as I can
tell.

When you pass a file descriptor to some one else, you give up reading,
writing and closing it as far as I can tell. Standard functions such as
fdopen() or fdopendir() make similar assumptions.

> 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.

POLLNVAL fails to solve the "file descriptor closed behind your back"
problem in at least two cases. First, it cannot cope if the poll() system
call was entered before the file descriptor is closed; the process will
deadlock instead. Second, it will obviously give totally incorrect results
if the file descriptor is reallocated to something else in the mean time,
with close() + something or with dup2(). 

> 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.

That looks much like a wrong design choice to me. poll() should always be
checked for POLLERR and POLLHUP, but POLLNVAL is best ignored in my
opinion. Alternatively, you can write a big "YOUR PROGRAM SUCK" message and
abort() because this is definitely a bug.

-- 
Rémi Denis-Courmont
http://www.remlab.net
http://fi.linkedin.com/in/remidenis