_xcb_conn_wait(): Do better at detecting closed sockets

Submitted by Adam Jackson on Oct. 13, 2016, 3:02 p.m.

Details

Message ID 20161013150246.9883-1-ajax@redhat.com
State New
Headers show
Series "_xcb_conn_wait(): Do better at detecting closed sockets" ( rev: 1 ) in XCB

Not browsing as part of any series.

Commit Message

Adam Jackson Oct. 13, 2016, 3:02 p.m.
Currently, when the X server crashes or a client is disconnected with
XKillClient, you get a somewhat confusing error message from libX11
along the lines of:

XIO:  fatal IO error 11 (Resource temporarily unavailable) on X server ":0"
      after 98 requests (40 known processed) with 0 events remaining.

What's happening here is the previous recvmsg has thrown EAGAIN, and
since errno is not cleared on success that's the errno that the I/O
error handler sees.

To fix this, check for POLLHUP explicitly, and if there's no more data
in the socket buffer to read, treat that as an error. We coerce errno to
EPIPE in this case, which triggers the existing EPIPE path in libX11 and
thus generates the much more honest error message:

X connection to :0 broken (explicit kill or server shutdown).

Signed-off-by: Adam Jackson <ajax@redhat.com>
---
 src/xcb_conn.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/src/xcb_conn.c b/src/xcb_conn.c
index 7d09637..ad94443 100644
--- a/src/xcb_conn.c
+++ b/src/xcb_conn.c
@@ -45,6 +45,7 @@ 
 #elif !defined _WIN32
 #include <sys/select.h>
 #endif
+#include <sys/ioctl.h>
 
 #ifdef _WIN32
 #include "xcb_windefs.h"
@@ -451,7 +452,7 @@  int _xcb_conn_wait(xcb_connection_t *c, pthread_cond_t *cond, struct iovec **vec
 #if USE_POLL
     memset(&fd, 0, sizeof(fd));
     fd.fd = c->fd;
-    fd.events = POLLIN;
+    fd.events = POLLIN | POLLHUP;
 #else
     FD_ZERO(&rfds);
     FD_SET(c->fd, &rfds);
@@ -484,6 +485,19 @@  int _xcb_conn_wait(xcb_connection_t *c, pthread_cond_t *cond, struct iovec **vec
             ret = -1;
             break;
         }
+
+	/* hangup with no data left to read is an error */
+	if (fd.revents & POLLHUP)
+	{
+	    int unread = -1;
+	    ioctl(c->fd, FIONREAD, &unread);
+	    if (unread <= 0)
+	    {
+		/* coerce errno to not be EAGAIN */
+		errno = EPIPE;
+		ret = -1;
+	    }
+	}
 #else
         ret = select(c->fd + 1, &rfds, &wfds, 0, 0);
 #endif

Comments

On Thu, Oct 13, 2016 at 9:02 AM, Adam Jackson <ajax@redhat.com> wrote:

> Currently, when the X server crashes or a client is disconnected with
> XKillClient, you get a somewhat confusing error message from libX11
> along the lines of:
>
> XIO:  fatal IO error 11 (Resource temporarily unavailable) on X server ":0"
>       after 98 requests (40 known processed) with 0 events remaining.
>
> I had been wondering about a similar although less verbose message from
gvim during server crash.


> What's happening here is the previous recvmsg has thrown EAGAIN, and
> since errno is not cleared on success that's the errno that the I/O
> error handler sees.
>
> excellent troubleshooting.


> To fix this, check for POLLHUP explicitly, and if there's no more data
> in the socket buffer to read, treat that as an error. We coerce errno to
> EPIPE in this case, which triggers the existing EPIPE path in libX11 and
> thus generates the much more honest error message:
>
> X connection to :0 broken (explicit kill or server shutdown).
>
> Signed-off-by: Adam Jackson <ajax@redhat.com>
> ---
>  src/xcb_conn.c | 16 +++++++++++++++-
>  1 file changed, 15 insertions(+), 1 deletion(-)
>
> diff --git a/src/xcb_conn.c b/src/xcb_conn.c
> index 7d09637..ad94443 100644
> --- a/src/xcb_conn.c
> +++ b/src/xcb_conn.c
> @@ -45,6 +45,7 @@
>  #elif !defined _WIN32
>  #include <sys/select.h>
>  #endif
> +#include <sys/ioctl.h>
>
>  #ifdef _WIN32
>  #include "xcb_windefs.h"
> @@ -451,7 +452,7 @@ int _xcb_conn_wait(xcb_connection_t *c,
> pthread_cond_t *cond, struct iovec **vec
>  #if USE_POLL
>      memset(&fd, 0, sizeof(fd));
>      fd.fd = c->fd;
> -    fd.events = POLLIN;
> +    fd.events = POLLIN | POLLHUP;
>  #else
>      FD_ZERO(&rfds);
>      FD_SET(c->fd, &rfds);
> @@ -484,6 +485,19 @@ int _xcb_conn_wait(xcb_connection_t *c,
> pthread_cond_t *cond, struct iovec **vec
>              ret = -1;
>              break;
>          }
> +
> +       /* hangup with no data left to read is an error */
> +       if (fd.revents & POLLHUP)
> +       {
> +           int unread = -1;
> +           ioctl(c->fd, FIONREAD, &unread);
> +           if (unread <= 0)
> +           {
> +               /* coerce errno to not be EAGAIN */
> +               errno = EPIPE;
> +               ret = -1;
> +           }
> +       }
>  #else
>          ret = select(c->fd + 1, &rfds, &wfds, 0, 0);
>  #endif
> --
> 2.9.3
>
> _______________________________________________
> Xcb mailing list
> Xcb@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/xcb