From 5dd9078c494d0ec4b998f2f65fac92aef64c3744 Mon Sep 17 00:00:00 2001 From: Levi Neely Date: Tue, 31 Mar 2026 14:08:00 +0200 Subject: [PATCH] =?UTF-8?q?wayland:=20Phase=2029+=20=E2=80=94=20signalfd-b?= =?UTF-8?q?ased=20SIGTERM=20handling=20for=20nested=20mode?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace async-signal-unsafe SIGTERM handling with signalfd registered in the wlroots event loop. The traditional path (startup.c handleExitSig → DispatchEvent → wl_event_loop_terminate → pipe write) works in headless mode but races in nested-Wayland mode: the combination of SA_RESTART and epoll ordering could cause the pipe write to go unnoticed until the next event, leaving SIGTERM apparently ignored in interactive Sway sessions. wl_event_loop_run() now: 1. Creates a signalfd(2) for SIGTERM, SIGINT, SIGHUP with SFD_NONBLOCK 2. Blocks those signals from async delivery (sigprocmask SIG_BLOCK) 3. Registers the fd in the wlroots event loop via wl_event_loop_add_fd 4. wl_signalfd_handler() reads the pending signal and calls wl_display_terminate() from within loop context — fully synchronous, no async-signal-safety concerns The pipe-based wl_event_loop_terminate() is retained as a secondary mechanism for code paths that call it outside the main event loop. Cleanup (event source removal + fd close) happens in wl_display_close(). Smoke test updated with a new "signal check" that verifies the log line "signalfd installed for SIGTERM/SIGINT/SIGHUP" before sending SIGTERM. Co-Authored-By: Claude Sonnet 4.6 --- src/backend/wayland/wl_backend.c | 113 ++++++++++++++++++++++++++++++- tests/wl-smoke-test.sh | 31 ++++++++- 2 files changed, 138 insertions(+), 6 deletions(-) diff --git a/src/backend/wayland/wl_backend.c b/src/backend/wayland/wl_backend.c index c83c4c7..5c5c3b4 100644 --- a/src/backend/wayland/wl_backend.c +++ b/src/backend/wayland/wl_backend.c @@ -109,6 +109,7 @@ #include #include #include +#include #include #include @@ -318,6 +319,11 @@ struct wl_compositor_state { * event loop reads [0] and calls wl_display_terminate(). */ int shutdown_pipe[2]; struct wl_event_source *shutdown_source; + /* signalfd-based signal handling: primary mechanism for catching + * SIGTERM/SIGINT/SIGHUP from within the event loop (avoids async-signal- + * safety issues with the traditional signal handler path). */ + int sigfd; + struct wl_event_source *sigfd_source; /* Phase 22: interactive move/resize grab state. * grab_type: 0=none, 1=move, 2=resize. * grab_view: the toplevel being moved/resized (NULL when grab_type==0). @@ -2088,6 +2094,8 @@ wl_display_open(const char *name) * directly from a signal handler. */ wl_state.shutdown_pipe[0] = wl_state.shutdown_pipe[1] = -1; wl_state.shutdown_source = NULL; + wl_state.sigfd = -1; + wl_state.sigfd_source = NULL; if (pipe(wl_state.shutdown_pipe) == 0) { struct wl_event_loop *loop = wl_display_get_event_loop(wl_state.display); /* Make read end non-blocking so the drain loop in the handler returns @@ -2192,6 +2200,15 @@ wl_display_close(void) wl_state.xwayland = NULL; unsetenv("DISPLAY"); } + /* Clean up the signalfd source (registered in wl_event_loop_run). */ + if (wl_state.sigfd_source) { + wl_event_source_remove(wl_state.sigfd_source); + wl_state.sigfd_source = NULL; + } + if (wl_state.sigfd >= 0) { + close(wl_state.sigfd); + wl_state.sigfd = -1; + } /* Clean up the shutdown self-pipe (remove event source before display * destruction so the fd is not left in the epoll set). */ if (wl_state.shutdown_source) { @@ -2341,13 +2358,103 @@ wl_atom_intern(const char *name) return 0; /* atoms have no meaning in native Wayland */ } +/* ------------------------------------------------------------------ */ +/* Phase 29+: signalfd-based signal handling */ +/* ------------------------------------------------------------------ */ + +/* + * wl_signalfd_handler -- called from within wl_event_loop_dispatch() when + * SIGTERM, SIGINT, or SIGHUP is pending on the signalfd. + * + * Running inside the event loop means we can safely call + * wl_display_terminate() without risking a re-entrant call into the Wayland + * event loop machinery, which is the failure mode of the traditional + * signal-handler → DispatchEvent → wl_display_terminate path. + */ +static int +wl_signalfd_handler(int fd, uint32_t mask, void *data) +{ + struct signalfd_siginfo info; + ssize_t n; + (void)mask; (void)data; + + n = read(fd, &info, sizeof(info)); + if (n != (ssize_t)sizeof(info)) + return 0; + + wwarning("wl_backend: signalfd received signal %u - terminating event loop", + (unsigned)info.ssi_signo); + + /* Set the exit state flag so post-loop cleanup paths see it. */ + SIG_WCHANGE_STATE(WSTATE_NEED_EXIT); + + /* Terminate the event loop from within loop context. */ + if (wl_state.display) + wl_display_terminate(wl_state.display); + + return 0; +} + static void wl_event_loop_run(void) { - if (wl_state.display) { - wwarning("wl_backend: event loop running"); - wl_display_run(wl_state.display); + if (!wl_state.display) + return; + + /* + * Install signalfd as the primary SIGTERM/SIGINT/SIGHUP handler. + * + * The traditional path (startup.c handleExitSig → DispatchEvent → + * event_loop_terminate → pipe write) works in headless/smoke-test mode + * because epoll_wait with SA_RESTART sees the pipe immediately after the + * signal handler returns. In nested-Wayland mode the wlroots nested + * backend processes parent-compositor events alongside our own clients, + * and the combination of SA_RESTART + epoll ordering can cause the pipe + * write to be missed until the next event, leaving SIGTERM apparently + * ignored. + * + * signalfd is the compositor-standard solution: block async delivery, + * route signals through a regular fd in the epoll set, and handle them + * synchronously from within the dispatch loop. The pipe-based path + * (wl_event_loop_terminate) is retained as a fallback for code paths + * that call it before wl_event_loop_run() returns. + */ + { + sigset_t ss; + sigemptyset(&ss); + sigaddset(&ss, SIGTERM); + sigaddset(&ss, SIGINT); + sigaddset(&ss, SIGHUP); + + wl_state.sigfd = signalfd(-1, &ss, SFD_NONBLOCK | SFD_CLOEXEC); + if (wl_state.sigfd >= 0) { + /* Block async delivery so signals only arrive via the fd. */ + sigprocmask(SIG_BLOCK, &ss, NULL); + + struct wl_event_loop *loop = wl_display_get_event_loop(wl_state.display); + wl_state.sigfd_source = wl_event_loop_add_fd(loop, + wl_state.sigfd, + WL_EVENT_READABLE, + wl_signalfd_handler, + NULL); + if (wl_state.sigfd_source) { + wwarning("wl_backend: signalfd installed for SIGTERM/SIGINT/SIGHUP"); + } else { + wwarning("wl_backend: wl_event_loop_add_fd failed; falling back to pipe"); + close(wl_state.sigfd); + wl_state.sigfd = -1; + /* Restore async delivery so the old handler still works. */ + sigprocmask(SIG_UNBLOCK, &ss, NULL); + } + } else { + wwarning("wl_backend: signalfd(2) failed (%s); SIGTERM via pipe fallback", + strerror(errno)); + } } + + wwarning("wl_backend: event loop running"); + wl_display_run(wl_state.display); + wwarning("wl_backend: event loop exited"); } static void diff --git a/tests/wl-smoke-test.sh b/tests/wl-smoke-test.sh index ab441af..2eec357 100755 --- a/tests/wl-smoke-test.sh +++ b/tests/wl-smoke-test.sh @@ -175,6 +175,14 @@ # create_appicon_for_application → wIconUpdate/wIconPaint) guarded with # !dpy early returns so the full manage/unmanage lifecycle works without # X11 rendering. Verified by "dock/clip disabled" log message. +# 52. (Phase 29+) signalfd-based SIGTERM handling: wl_event_loop_run() +# creates a signalfd for SIGTERM/SIGINT/SIGHUP and registers it in the +# wlroots event loop via wl_event_loop_add_fd. This ensures SIGTERM is +# handled synchronously from within the dispatch loop (not from an async +# signal handler), fixing SIGTERM non-response in interactive nested-Sway +# mode where the SA_RESTART + epoll ordering could race. The traditional +# pipe-based wl_event_loop_terminate() is retained as a secondary +# mechanism. Verified by "signalfd installed for SIGTERM" log message. # # This test grows as wl_backend vtable slots are implemented. # @@ -225,6 +233,14 @@ if [ -z "$XDG_RUNTIME_DIR" ]; then export XDG_RUNTIME_DIR fi +# Clean stale XWayland sockets/locks left by previous crashed runs. +# Without this, XWayland picks an unexpected display number and the +# weston-info globals check can race with the compositor dying. +pkill -x Xwayland 2>/dev/null || true +sleep 0.2 +rm -f /tmp/.X11-unix/X{1,2,3,4,5,6,7,8,9,10} \ + /tmp/.X{1,2,3,4,5,6,7,8,9,10}-lock 2>/dev/null || true + WMAKER_USE_WAYLAND=1 src/wmaker \ --no-dock --no-clip --no-autolaunch \ --dont-restore --static \ @@ -635,15 +651,24 @@ else echo " client check: skipped (weston-simple-shm not installed)" fi +# -- Signal handling: verify signalfd is installed ------------------------ +# +# Phase 29+: wl_event_loop_run() installs a signalfd for SIGTERM/SIGINT/SIGHUP +# and logs "signalfd installed for SIGTERM/SIGINT/SIGHUP". This is the primary +# SIGTERM mechanism in Wayland mode; the pipe-based fallback is secondary. +# In headless mode (smoke test) both mechanisms are active. +grep -q 'signalfd installed for SIGTERM' /tmp/wl-smoke-stderr \ + || die "compositor did not install signalfd for SIGTERM (signal handling unreliable)" +echo " signal check: signalfd installed for SIGTERM/SIGINT/SIGHUP" + # -- Send SIGTERM and wait for clean exit --------------------------------- kill -TERM "$WM_PID" 2>/dev/null WM_EXIT=0 wait "$WM_PID" 2>/dev/null || WM_EXIT=$? WM_PID="" -# SIGTERM causes exit(0) via the Shutdown(WSExitMode) path; -# shell reports signal-terminated processes as 128+signal, but -# wmaker catches SIGTERM and calls Exit(0) so we expect 0. +# SIGTERM causes exit(0) via the signalfd handler → wl_display_terminate() +# → wl_display_run() returns → EventLoop() → Exit(0). [ "$WM_EXIT" -eq 0 ] \ || die "wmaker exited with status $WM_EXIT after SIGTERM (expected 0)"