x11: gracefully handle X11 connection error

Perform X11 connection recovery via XSetIOErrorExitHandler mechanism.

Implementation is largely inspired by this change to GNOME/mutter
https://gitlab.gnome.org/GNOME/mutter/-/merge_requests/1447

Part-of: <https://gitlab.freedesktop.org/pulseaudio/pulseaudio/-/merge_requests/441>

diff --git a/configure.ac b/configure.ac
index 357b6d7..bd3a533 100644 (file)
--- a/configure.ac
+++ b/configure.ac
@@ -633,8 +633,14 @@ AC_ARG_ENABLE([x11],
     AS_HELP_STRING([--disable-x11],[Disable optional X11 support]))
 
 AS_IF([test "x$enable_x11" != "xno"],
-    [PKG_CHECK_MODULES(X11, [ x11-xcb xcb >= 1.6 ice sm xtst ], HAVE_X11=1, HAVE_X11=0)],
-    HAVE_X11=0)
+    [
+        PKG_CHECK_MODULES(X11, [ x11-xcb xcb >= 1.6 ice sm xtst ], HAVE_X11=1, HAVE_X11=0)
+        AC_CHECK_LIB(X11, XSetIOErrorExitHandler, [HAVE_XSETIOERROREXITHANDLER=yes], [HAVE_XSETIOERROREXITHANDLER=no])
+    ],
+    [
+        HAVE_X11=0
+        HAVE_XSETIOERROREXITHANDLER=no
+    ])
 
 AS_IF([test "x$enable_x11" = "xyes" && test "x$HAVE_X11" = "x0"],
     [AC_MSG_ERROR([*** X11 not found])])
@@ -642,6 +648,7 @@ AS_IF([test "x$enable_x11" = "xyes" && test "x$HAVE_X11" = "x0"],
 AC_SUBST(HAVE_X11)
 AM_CONDITIONAL([HAVE_X11], [test "x$HAVE_X11" = x1])
 AS_IF([test "x$HAVE_X11" = "x1"], AC_DEFINE([HAVE_X11], 1, [Have X11?]))
+AS_IF([test "x$HAVE_XSETIOERROREXITHANDLER" = "xyes"], AC_DEFINE([HAVE_XSETIOERROREXITHANDLER], 1, [Have XSetIOErrorExitHandler function.]))
 
 #### Capabilities (optional) ####
 
@@ -1687,6 +1694,7 @@ echo "
 
     Enable memfd shared memory:    ${ENABLE_MEMFD}
     Enable X11:                    ${ENABLE_X11}
+      Safe X11 I/O errors:         ${HAVE_XSETIOERROREXITHANDLER}
     Enable OSS Output:             ${ENABLE_OSS_OUTPUT}
     Enable OSS Wrapper:            ${ENABLE_OSS_WRAPPER}
     Enable EsounD:                 ${ENABLE_ESOUND}

diff --git a/meson.build b/meson.build
index cd617c6..23e04ef 100644 (file)
--- a/meson.build
+++ b/meson.build
@@ -650,6 +650,9 @@ if x11_dep.found()
   sm_dep   = dependency('sm',   required : true)
   xtst_dep = dependency('xtst', required : true)
   cdata.set('HAVE_X11', 1)
+  if cc.has_function('XSetIOErrorExitHandler', dependencies: x11_dep)
+    cdata.set('HAVE_XSETIOERROREXITHANDLER', 1)
+  endif
 endif
 
 # Module dependencies
@@ -847,6 +850,7 @@ summary = [
   '',
   'Enable memfd shared memory:    @0@'.format(cdata.has('HAVE_MEMFD')),
   'Enable X11:                    @0@'.format(x11_dep.found()),
+  '  Safe X11 I/O errors:         @0@'.format(cdata.has('HAVE_XSETIOERROREXITHANDLER')),
   'Enable OSS Output:             @0@'.format(cdata.has('HAVE_OSS_OUTPUT')),
   'Enable OSS Wrapper:            @0@'.format(cdata.has('HAVE_OSS_WRAPPER')),
 #  'Enable EsounD:                 @0@'.format(${ENABLE_ESOUND}),

diff --git a/src/modules/x11/module-x11-bell.c b/src/modules/x11/module-x11-bell.c
index eab1e6c..058a933 100644 (file)
--- a/src/modules/x11/module-x11-bell.c
+++ b/src/modules/x11/module-x11-bell.c
@@ -93,6 +93,8 @@ static void x11_kill_cb(pa_x11_wrapper *w, void *userdata) {
     pa_assert(u);
     pa_assert(u->x11_wrapper == w);
 
+    pa_log_debug("X11 client kill callback called");
+
     if (u->x11_client)
         pa_x11_client_free(u->x11_client);
 

diff --git a/src/modules/x11/module-x11-cork-request.c b/src/modules/x11/module-x11-cork-request.c
index 6b1a86b..b9378f5 100644 (file)
--- a/src/modules/x11/module-x11-cork-request.c
+++ b/src/modules/x11/module-x11-cork-request.c
@@ -66,6 +66,8 @@ static void x11_kill_cb(pa_x11_wrapper *w, void *userdata) {
     pa_assert(u);
     pa_assert(u->x11_wrapper == w);
 
+    pa_log_debug("X11 client kill callback called");
+
     if (u->x11_client) {
         pa_x11_client_free(u->x11_client);
         u->x11_client = NULL;

diff --git a/src/modules/x11/module-x11-publish.c b/src/modules/x11/module-x11-publish.c
index 68adf15..da619f8 100644 (file)
--- a/src/modules/x11/module-x11-publish.c
+++ b/src/modules/x11/module-x11-publish.c
@@ -116,6 +116,8 @@ static void x11_kill_cb(pa_x11_wrapper *w, void *userdata) {
     pa_assert(u);
     pa_assert(u->x11_wrapper == w);
 
+    pa_log_debug("X11 client kill callback called");
+
     if (u->x11_client)
         pa_x11_client_free(u->x11_client);
 

diff --git a/src/modules/x11/module-x11-xsmp.c b/src/modules/x11/module-x11-xsmp.c
index 459da13..eb18b43 100644 (file)
--- a/src/modules/x11/module-x11-xsmp.c
+++ b/src/modules/x11/module-x11-xsmp.c
@@ -64,7 +64,7 @@ static void die_cb(SmcConn connection, SmPointer client_data) {
 
     pa_log_debug("Got die message from XSMP.");
 
-    pa_x11_wrapper_kill(u->x11);
+    pa_x11_wrapper_kill_deferred(u->x11);
 
     pa_x11_wrapper_unref(u->x11);
     u->x11 = NULL;

diff --git a/src/pulsecore/x11wrap.c b/src/pulsecore/x11wrap.c
index 0c040cf..635d21e 100644 (file)
--- a/src/pulsecore/x11wrap.c
+++ b/src/pulsecore/x11wrap.c
@@ -33,6 +33,8 @@
 
 #include "x11wrap.h"
 
+#include <X11/Xlib.h>
+
 typedef struct pa_x11_internal pa_x11_internal;
 
 struct pa_x11_internal {
@@ -51,6 +53,7 @@ struct pa_x11_wrapper {
 
     pa_defer_event* defer_event;
     pa_io_event* io_event;
+    pa_defer_event* cleanup_event;
 
     PA_LLIST_HEAD(pa_x11_client, clients);
     PA_LLIST_HEAD(pa_x11_internal, internals);
@@ -64,6 +67,8 @@ struct pa_x11_client {
     void *userdata;
 };
 
+static void x11_wrapper_kill(pa_x11_wrapper *w);
+
 /* Dispatch all pending X11 events */
 static void work(pa_x11_wrapper *w) {
     pa_assert(w);
@@ -167,6 +172,38 @@ static void x11_watch(Display *display, XPointer userdata, int fd, Bool opening,
         x11_internal_remove(w, (pa_x11_internal*) *watch_data);
 }
 
+static int x11_error_handler(Display* display, XErrorEvent* error_event) {
+    pa_log_warn("X11 error handler called");
+    return 0;
+}
+
+static int x11_io_error_handler(Display* display) {
+    pa_log_warn("X11 I/O error handler called");
+    return 0;
+}
+
+static void deferred_x11_teardown(pa_mainloop_api *m, pa_defer_event *e, void *userdata) {
+    pa_x11_wrapper *w = userdata;
+
+    m->defer_enable(e, 0);
+
+    pa_log_debug("Start tearing down X11 modules after X11 I/O error");
+
+    x11_wrapper_kill(w);
+
+    pa_log_debug("Done tearing down X11 modules after X11 I/O error");
+}
+
+#ifdef HAVE_XSETIOERROREXITHANDLER
+static void x11_io_error_exit_handler(Display* display, void *userdata) {
+    pa_x11_wrapper *w = userdata;
+
+    pa_log_warn("X11 I/O error exit handler called, preparing to tear down X11 modules");
+
+    pa_x11_wrapper_kill_deferred(w);
+}
+#endif
+
 static pa_x11_wrapper* x11_wrapper_new(pa_core *c, const char *name, const char *t) {
     pa_x11_wrapper*w;
     Display *d;
@@ -187,11 +224,20 @@ static pa_x11_wrapper* x11_wrapper_new(pa_core *c, const char *name, const char
 
     w->defer_event = c->mainloop->defer_new(c->mainloop, defer_event, w);
     w->io_event = c->mainloop->io_new(c->mainloop, ConnectionNumber(d), PA_IO_EVENT_INPUT, display_io_event, w);
+    w->cleanup_event = c->mainloop->defer_new(c->mainloop, deferred_x11_teardown, w);
+    w->core->mainloop->defer_enable(w->cleanup_event, 0);
 
+    XSetErrorHandler(x11_error_handler);
+    XSetIOErrorHandler(x11_io_error_handler);
+#ifdef HAVE_XSETIOERROREXITHANDLER
+    XSetIOErrorExitHandler(d, x11_io_error_exit_handler, w);
+#endif
     XAddConnectionWatch(d, x11_watch, (XPointer) w);
 
     pa_assert_se(pa_shared_set(c, w->property_name, w) >= 0);
 
+    pa_log_debug("Created X11 connection wrapper '%s'", w->property_name);
+
     return w;
 }
 
@@ -202,9 +248,12 @@ static void x11_wrapper_free(pa_x11_wrapper*w) {
 
     pa_assert(!w->clients);
 
+    pa_log_debug("Destroying X11 connection wrapper '%s'", w->property_name);
+
     XRemoveConnectionWatch(w->display, x11_watch, (XPointer) w);
     XCloseDisplay(w->display);
 
+    w->core->mainloop->defer_free(w->cleanup_event);
     w->core->mainloop->io_free(w->io_event);
     w->core->mainloop->defer_free(w->defer_event);
 
@@ -261,7 +310,15 @@ xcb_connection_t *pa_x11_wrapper_get_xcb_connection(pa_x11_wrapper *w) {
     return XGetXCBConnection(pa_x11_wrapper_get_display(w));
 }
 
-void pa_x11_wrapper_kill(pa_x11_wrapper *w) {
+void pa_x11_wrapper_kill_deferred(pa_x11_wrapper *w) {
+    pa_assert(w);
+
+    /* schedule X11 display teardown */
+    w->core->mainloop->defer_enable(w->cleanup_event, 1);
+}
+
+/* Kill the connection to the X11 display */
+static void x11_wrapper_kill(pa_x11_wrapper *w) {
     pa_x11_client *c, *n;
 
     pa_assert(w);

diff --git a/src/pulsecore/x11wrap.h b/src/pulsecore/x11wrap.h
index 0539303..2d81705 100644 (file)
--- a/src/pulsecore/x11wrap.h
+++ b/src/pulsecore/x11wrap.h
@@ -48,8 +48,8 @@ Display *pa_x11_wrapper_get_display(pa_x11_wrapper *w);
 /* Return the XCB connection object for this connection */
 xcb_connection_t *pa_x11_wrapper_get_xcb_connection(pa_x11_wrapper *w);
 
-/* Kill the connection to the X11 display */
-void pa_x11_wrapper_kill(pa_x11_wrapper *w);
+/* Initiate X11 connection teardown. */
+void pa_x11_wrapper_kill_deferred(pa_x11_wrapper *w);
 
 /* Register an X11 client, that is called for each X11 event */
 pa_x11_client* pa_x11_client_new(pa_x11_wrapper *w, pa_x11_event_cb_t event_cb, pa_x11_kill_cb_t kill_cb, void *userdata);