ahamilton/kick
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix SIGTERM shutdown deadlock: run teardown from an event-loop signal source

Browse source 
SIGTERM/SIGINT ran shutdown_cleanup() straight from a raw signal handler.
The teardown path calls malloc/free and libwayland proxy destroys, which
are not async-signal-safe: a SIGTERM landing inside launch_client()'s
fork() left libc's allocator lock held, and bridge_cleanup()'s calloc()
then blocked on it forever, so the compositor never exited and tests that
send SIGTERM timed out.

Route SIGTERM/SIGINT through wl_event_loop_add_signal() (signalfd) so the
teardown runs from a normal event-loop callback, where no allocator or
stdio lock can be held across it.  Also make sigchld_handler() async-signal-
safe (snprintf+write instead of fprintf, which takes the FILE lock).
This commit is contained in:
Andrew Hamilton 2026-08-11 19:07:30 +10:00
parent 8a46e92e03
commit 3cc2ab38fd
1 changed files with 27 additions and 6 deletions
Download patch file Download diff file Expand all files Collapse all files

33
tabbed-wlroots.c
View file

@ -2755,8 +2755,16 @@ static void sigchld_handler(int sig) {
int status;
pid_t pid;
if (tabbed_log_level() >= TABBED_LOG_DEBUG) {
while ((pid = waitpid(-1, &status, WNOHANG)) > 0)
fprintf(stderr, "[child] pid %d exited status 0x%x\n", pid, status);
/* snprintf()/write() are async-signal-safe; fprintf() is not (it
takes the FILE lock, which can deadlock if we interrupt a logging
fprintf in the main loop). */
char buf[64];
while ((pid = waitpid(-1, &status, WNOHANG)) > 0) {
int n = snprintf(buf, sizeof(buf),
"[child] pid %d exited status 0x%x\n", pid, status);
if (n > 0 && (size_t)n < sizeof(buf))
(void)!write(STDERR_FILENO, buf, (size_t)n);
}
} else {
while (waitpid(-1, &status, WNOHANG) > 0)
;
@ -2765,11 +2773,21 @@ static void sigchld_handler(int sig) {
/* On SIGTERM or Ctrl-C (SIGINT), take down the clients and exit cleanly
rather than leaving them orphaned when the compositor dies. The clients
live in their own process groups, so a terminal Ctrl-C only reaches us. */
static void close_signal_handler(int sig) {
live in their own process groups, so a terminal Ctrl-C only reaches us.
These signals are delivered through the event loop (wl_event_loop_add_signal
in main) rather than a raw sigaction handler: the full teardown calls
malloc/free and libwayland proxy destroys, which are not async-signal-safe
and deadlock if the signal interrupts fork()/malloc()/logging (observed:
SIGTERM landing inside launch_client's fork() left libc's allocator lock
held, and shutdown_cleanup() -> bridge_cleanup() -> calloc() blocked on it
forever). Running it from a normal loop callback makes that impossible. */
static int close_signal_cb(int sig, void *data) {
(void)sig;
(void)data;
shutdown_cleanup();
_exit(0);
return 0;
}
/* SIGUSR1: save the current scene buffer and host back buffer on the next
@ -2804,8 +2822,6 @@ int main(int argc, char *argv[]) {
sigaction(SIGSEGV, &crash_sa, NULL);
sigaction(SIGABRT, &crash_sa, NULL);
signal(SIGCHLD, sigchld_handler);
signal(SIGTERM, close_signal_handler);
signal(SIGINT, close_signal_handler);
signal(SIGUSR1, sigusr1_handler);
/* wlroots logs follow TABBED_LOG: quiet keeps only errors, info drops
@ -2829,6 +2845,11 @@ int main(int argc, char *argv[]) {
}
server.loop = wl_display_get_event_loop(server.wl_display);
/* Route SIGTERM/SIGINT through the event loop (signalfd) so shutdown runs
in normal context; see close_signal_cb for why a raw handler is unsafe. */
wl_event_loop_add_signal(server.loop, SIGTERM, close_signal_cb, NULL);
wl_event_loop_add_signal(server.loop, SIGINT, close_signal_cb, NULL);
/* stdin event feed: lets tests/drivers inject raw keyboard events
into the real host key handlers without a host compositor. */
server.ctl_source = wl_event_loop_add_fd(server.loop, STDIN_FILENO,