[Xcb] [PATCH] _xcb_conn_wait: Shut down the connection on unexpected poll() events.

Jamey Sharp jamey at minilop.net
Tue Jul 13 10:19:00 PDT 2010


If a client calls close(2) on the connection's file descriptor and then
flushes writes, libxcb causes a hang in the client.

Any flush 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 a
client hang.

XTS testcase Xlib3/XConnectionNumber triggers this bug. It creates a
display connection, closes its file descriptor, tries to send a no-op,
and then expects an error.
http://cgit.freedesktop.org/xorg/test/xts/tree/xts5/Xlib3/XConnectionNumber.m

If poll returned POLLHUP or POLLERR, we might see the same result.

If poll returns any event we didn't ask for, this patch causes
_xcb_conn_shutdown() to be invoked and an error 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.

I believe this fix both is safe and will handle any similar error. POSIX
says that the only bits poll is permitted to set in revents are those
bits that were set in events, plus POLLHUP, POLLERR, and POLLNVAL. So if
we see any flags we didn't ask for then something has gone wrong.

Patch inspired by earlier proposals from Peter Hutterer and Aaron
Plattner--thanks!

Reported-by: Peter Hutterer <peter.hutterer at who-t.net>
Reported-by: Aaron Plattner <aplattner at nvidia.com>
Signed-off-by: Jamey Sharp <jamey at minilop.net>
Cc: Peter Hutterer <peter.hutterer at who-t.net>
Cc: Dan Nicholson <dbn.lists at gmail.com>
Cc: Aaron Plattner <aplattner at nvidia.com>
---
I think this is the patch I want, but I've only compile-tested it. Can I
get tested-by or reviewed-by tags from you?

 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 7e18891..04e0430 100644
--- a/src/xcb_conn.c
+++ b/src/xcb_conn.c
@@ -311,6 +311,10 @@ int _xcb_conn_wait(xcb_connection_t *c, pthread_cond_t *cond, struct iovec **vec
     do {
 #if USE_POLL
         ret = poll(&fd, 1, -1);
+        /* If poll() returns an event we didn't expect, such as
+         * POLLNVAL, treat it as if it failed. */
+        if(ret >= 0 && (fd.revents & ~fd.events))
+            ret = -1;
 #else
         ret = select(c->fd + 1, &rfds, &wfds, 0, 0);
 #endif
-- 
1.7.0



More information about the Xcb mailing list