[pulseaudio-commits] [Git][pulseaudio/pulseaudio][master] 2 commits: x11: gracefully handle X11 connection error
PulseAudio Marge Bot
gitlab at gitlab.freedesktop.org
Mon Jan 4 16:05:07 UTC 2021
PulseAudio Marge Bot pushed to branch master at PulseAudio / pulseaudio
Commits:
b6396dbe by Igor V. Kovalenko at 2021-01-04T15:59:22+00:00
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>
- - - - -
6bfd9c80 by Igor V. Kovalenko at 2021-01-04T15:59:22+00:00
x11: gracefully handle ICE connection error
Implementation is largely inspired by GNOME/mutter ICE session handling.
Part-of: <https://gitlab.freedesktop.org/pulseaudio/pulseaudio/-/merge_requests/441>
- - - - -
8 changed files:
- configure.ac
- meson.build
- src/modules/x11/module-x11-bell.c
- src/modules/x11/module-x11-cork-request.c
- src/modules/x11/module-x11-publish.c
- src/modules/x11/module-x11-xsmp.c
- src/pulsecore/x11wrap.c
- src/pulsecore/x11wrap.h
Changes:
=====================================
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}
=====================================
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}),
=====================================
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);
=====================================
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;
=====================================
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);
=====================================
src/modules/x11/module-x11-xsmp.c
=====================================
@@ -55,21 +55,50 @@ struct userdata {
pa_module *module;
pa_client *client;
SmcConn connection;
- pa_x11_wrapper *x11;
+
+ pa_x11_wrapper *x11_wrapper;
+ pa_x11_client *x11_client;
};
+static void x11_kill_cb(pa_x11_wrapper *w, void *userdata) {
+ struct userdata *u = userdata;
+
+ pa_assert(w);
+ pa_assert(u);
+ pa_assert(u->x11_wrapper == w);
+
+ pa_log_debug("X11 client kill callback called");
+
+ if (u->connection) {
+ SmcCloseConnection(u->connection, 0, NULL);
+ u->connection = NULL;
+ }
+
+ if (u->x11_client) {
+ pa_x11_client_free(u->x11_client);
+ u->x11_client = NULL;
+ }
+
+ if (u->x11_wrapper) {
+ pa_x11_wrapper_unref(u->x11_wrapper);
+ u->x11_wrapper = NULL;
+ }
+
+ pa_module_unload_request(u->module, true);
+}
+
static void die_cb(SmcConn connection, SmPointer client_data) {
struct userdata *u = client_data;
pa_assert(u);
pa_log_debug("Got die message from XSMP.");
- pa_x11_wrapper_kill(u->x11);
-
- pa_x11_wrapper_unref(u->x11);
- u->x11 = NULL;
+ if (u->connection) {
+ SmcCloseConnection(u->connection, 0, NULL);
+ u->connection = NULL;
+ }
- pa_module_unload_request(u->module, true);
+ pa_x11_wrapper_kill_deferred(u->x11_wrapper);
}
static void save_complete_cb(SmcConn connection, SmPointer client_data) {
@@ -87,6 +116,7 @@ static void ice_io_cb(pa_mainloop_api*a, pa_io_event *e, int fd, pa_io_event_fla
IceConn connection = userdata;
if (IceProcessMessages(connection, NULL, NULL) == IceProcessMessagesIOError) {
+ pa_log_debug("IceProcessMessages: I/O error, closing ICE connection");
IceSetShutdownNegotiation(connection, False);
IceCloseConnection(connection);
}
@@ -106,6 +136,17 @@ static void new_ice_connection(IceConn connection, IcePointer client_data, Bool
c->mainloop->io_free(*watch_data);
}
+static IceIOErrorHandler ice_installed_handler;
+
+/* We call any handler installed before (or after) module is loaded but
+ avoid calling the default libICE handler which does an exit() */
+
+static void ice_io_error_handler(IceConn iceConn) {
+ pa_log_warn("ICE I/O error handler called");
+ if (ice_installed_handler)
+ (*ice_installed_handler) (iceConn);
+}
+
int pa__init(pa_module*m) {
pa_modargs *ma = NULL;
@@ -123,17 +164,27 @@ int pa__init(pa_module*m) {
if (ice_in_use) {
pa_log("module-x11-xsmp may not be loaded twice.");
return -1;
- }
+ } else {
+ IceIOErrorHandler default_handler;
+
+ ice_installed_handler = IceSetIOErrorHandler (NULL);
+ default_handler = IceSetIOErrorHandler (ice_io_error_handler);
- IceAddConnectionWatch(new_ice_connection, m->core);
- ice_in_use = true;
+ if (ice_installed_handler == default_handler)
+ ice_installed_handler = NULL;
+
+ IceSetIOErrorHandler(ice_io_error_handler);
+
+ IceAddConnectionWatch(new_ice_connection, m->core);
+ ice_in_use = true;
+ }
m->userdata = u = pa_xnew(struct userdata, 1);
u->core = m->core;
u->module = m;
u->client = NULL;
u->connection = NULL;
- u->x11 = NULL;
+ u->x11_wrapper = NULL;
if (!(ma = pa_modargs_new(m->argument, valid_modargs))) {
pa_log("Failed to parse module arguments");
@@ -147,9 +198,11 @@ int pa__init(pa_module*m) {
}
}
- if (!(u->x11 = pa_x11_wrapper_get(m->core, pa_modargs_get_value(ma, "display", NULL))))
+ if (!(u->x11_wrapper = pa_x11_wrapper_get(m->core, pa_modargs_get_value(ma, "display", NULL))))
goto fail;
+ u->x11_client = pa_x11_client_new(u->x11_wrapper, NULL, x11_kill_cb, u);
+
e = pa_modargs_get_value(ma, "session_manager", NULL);
if (!e && !getenv("SESSION_MANAGER")) {
@@ -253,8 +306,11 @@ void pa__done(pa_module*m) {
if (u->client)
pa_client_free(u->client);
- if (u->x11)
- pa_x11_wrapper_unref(u->x11);
+ if (u->x11_client)
+ pa_x11_client_free(u->x11_client);
+
+ if (u->x11_wrapper)
+ pa_x11_wrapper_unref(u->x11_wrapper);
pa_xfree(u);
}
=====================================
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);
=====================================
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);
View it on GitLab: https://gitlab.freedesktop.org/pulseaudio/pulseaudio/-/compare/28f646fd87e824ba5e89377cd9aedde74e6be96c...6bfd9c809e1a679043c6c4bfdd1e117ed7b5fc2a
--
View it on GitLab: https://gitlab.freedesktop.org/pulseaudio/pulseaudio/-/compare/28f646fd87e824ba5e89377cd9aedde74e6be96c...6bfd9c809e1a679043c6c4bfdd1e117ed7b5fc2a
You're receiving this email because of your account on gitlab.freedesktop.org.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/pulseaudio-commits/attachments/20210104/b10ed3f1/attachment-0001.htm>
More information about the pulseaudio-commits
mailing list