From c9612225e10a5e1a9805e23dbbb3bd8c996255fd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Barnab=C3=A1s=20P=C5=91cze?= Date: Thu, 17 Feb 2022 19:39:30 +0100 Subject: [PATCH] pipewire: module-x11-bell: handle X11 errors Unfortunately, libX11 has global error and I/O error handlers, which make it inconvenient to use them from library code. Since libX11 1.7.0, there is a per-display "exit_handler" which is called from `_XIOError()`, however, the global I/O error handler is still called before that, and that - by default - calls `exit(1)` in `_XDefaultIOError()` after printing the error to stderr. To avoid exiting, custom handlers will be registered when libpipewire-module-x11-bell.so is loaded given that at that moment the default handlers are installed. When the shared object is unloaded, the handlers will be reset to the default ones given that the currently registered handlers are the ones that were registered when the module was loaded. The logic only works correctly if there are no concurrent calls to `XSet{IO}ErrorHandler()` while `{set,restore}_x11_handlers()` is running. Since module-x11-bell is probably mostly going to be loaded in `pipewire-pulse`, this seems like a reasonable assumption to make. --- meson.build | 28 +++++++------ src/modules/module-x11-bell.c | 74 ++++++++++++++++++++++++++++++++++- 2 files changed, 88 insertions(+), 14 deletions(-) diff --git a/meson.build b/meson.build index 440845e37..ac4ff9d51 100644 --- a/meson.build +++ b/meson.build @@ -221,18 +221,6 @@ foreach h : check_headers cdata.set(h.get(1), cc.has_header(h.get(0))) endforeach -check_functions = [ - ['gettid', '#include ', ['-D_GNU_SOURCE']], - ['memfd_create', '#include ', ['-D_GNU_SOURCE']], - ['getrandom', '#include \n#include ', ['-D_GNU_SOURCE']], - ['sigabbrev_np', '#include ', ['-D_GNU_SOURCE']], -] - -foreach f : check_functions - cdata.set('HAVE_' + f.get(0).to_upper(), - cc.has_function(f.get(0), prefix: f.get(1), args: f.get(2))) -endforeach - cdata.set('HAVE_PIDFD_OPEN', cc.get_define('SYS_pidfd_open', prefix: '#include ') != '') @@ -380,6 +368,22 @@ lilv_lib = dependency('lilv-0', required: get_option('lv2')) summary({'lilv (for lv2 plugins)': lilv_lib.found()}, bool_yn: true) cdata.set('HAVE_LILV', lilv_lib.found()) +check_functions = [ + ['gettid', '#include ', ['-D_GNU_SOURCE'], []], + ['memfd_create', '#include ', ['-D_GNU_SOURCE'], []], + ['getrandom', '#include \n#include ', ['-D_GNU_SOURCE'], []], + ['sigabbrev_np', '#include ', ['-D_GNU_SOURCE'], []], + ['XSetIOErrorExitHandler', '#include ', [], [x11_dep]], +] + +foreach f : check_functions + cdata.set('HAVE_' + f.get(0).to_upper(), + cc.has_function(f.get(0), + prefix: f.get(1), + args: f.get(2), + dependencies: f.get(3))) +endforeach + installed_tests_metadir = pipewire_datadir / 'installed-tests' / pipewire_name installed_tests_execdir = pipewire_libexecdir / 'installed-tests' / pipewire_name installed_tests_enabled = get_option('installed_tests').allowed() diff --git a/src/modules/module-x11-bell.c b/src/modules/module-x11-bell.c index 44725c20e..cf73f1d0c 100644 --- a/src/modules/module-x11-bell.c +++ b/src/modules/module-x11-bell.c @@ -40,8 +40,9 @@ #include -#include "pipewire/pipewire.h" -#include "pipewire/impl.h" +#include +#include +#include /** \page page_module_x11_bell PipeWire Module: X11 Bell * @@ -153,6 +154,22 @@ static void display_io(void *data, int fd, uint32_t mask) } } +#ifdef HAVE_XSETIOERROREXITHANDLER +static void x11_io_error_exit_handler(Display *display, void *data) +{ + struct impl *impl = data; + + spa_assert(display == impl->display); + + pw_log_warn("X11 display (%s) has encountered a fatal I/O error", DisplayString(display)); + + pw_loop_destroy_source(impl->loop, impl->source); + impl->source = NULL; + + pw_impl_module_schedule_destroy(impl->module); +} +#endif + static int x11_connect(struct impl *impl, const char *name) { int major, minor; @@ -169,6 +186,10 @@ static int x11_connect(struct impl *impl, const char *name) if (!impl->source) return -errno; +#ifdef HAVE_XSETIOERROREXITHANDLER + XSetIOErrorExitHandler(impl->display, x11_io_error_exit_handler, impl); +#endif + major = XkbMajorVersion; minor = XkbMinorVersion; @@ -285,3 +306,52 @@ error: return res; } + +static int x11_error_handler(Display *display, XErrorEvent *error) +{ + pw_log_warn("X11 error handler called on display %s with error %d", + DisplayString(display), error->error_code); + return 0; +} + +static int x11_io_error_handler(Display *display) +{ + pw_log_warn("X11 I/O error handler called on display %s", DisplayString(display)); + return 0; +} + +__attribute__((constructor)) +static void set_x11_handlers(void) +{ + { + XErrorHandler prev = XSetErrorHandler(NULL); + XErrorHandler def = XSetErrorHandler(x11_error_handler); + + if (prev != def) + XSetErrorHandler(prev); + } + + { + XIOErrorHandler prev = XSetIOErrorHandler(NULL); + XIOErrorHandler def = XSetIOErrorHandler(x11_io_error_handler); + + if (prev != def) + XSetIOErrorHandler(prev); + } +} + +__attribute__((destructor)) +static void restore_x11_handlers(void) +{ + { + XErrorHandler prev = XSetErrorHandler(NULL); + if (prev != x11_error_handler) + XSetErrorHandler(prev); + } + + { + XIOErrorHandler prev = XSetIOErrorHandler(NULL); + if (prev != x11_io_error_handler) + XSetIOErrorHandler(prev); + } +}