lkn
/
windowmaker-wl
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

wayland: Phase 19 — clean SIGTERM shutdown (smoke test 22/22 PASS, exit 0) 

Three bugs in the shutdown path caused wmaker to exit with SIGABRT (134)
or hang on SIGTERM when running the Wayland backend:

1. wl_display_destroy() SIGABRT — wlroots 0.15 double-remove bug
   wlroots registers internal listeners on the XWayland wl_client's destroy
   signal.  Those listeners self-remove during client destruction.
   wlr_xwayland_server_destroy() then tries wl_list_remove() on the same
   already-nulled links, corrupting the heap → SIGABRT.
   Fix: skip wl_display_destroy() entirely; Exit() calls exit(0) immediately
   after wl_display_close() returns, so the kernel reclaims all resources.

2. wl_display_destroy_clients() hang
   wlroots' XWayland client destroy handler performs blocking cleanup
   that can hang when Xwayland is in mid-dispatch.
   Fix: skip wl_display_destroy_clients() for the same reason as (1).

3. Global listener leak — dangling wl_listener.link pointers
   All globally-registered wl_listeners (output frame, backend new_output,
   xdg_shell new_surface, decoration, xwayland new_surface, keyboard key/
   modifiers, pointer motion/button/frame) were never removed before the
   owning wlroots objects were freed.  This left link.prev/next dangling
   into freed signal structs, causing heap corruption in later allocations.
   Fix: WL_LISTENER_REMOVE() all 11 global listeners in wl_display_close()
   before any display teardown.

Additional fixes included in this phase:
- handle_toplevel_destroy: remove all 10 per-view listeners (was only 4),
  preventing dangling pointers after each client session.
- handle_output_frame: guard on wl_state.allocator; call
  wlr_scene_output_send_frame_done() when no GPU allocator so frame
  callbacks still fire (fixes weston-simple-shm stall on headless output).
- handle_backend_new_output: call wlr_output_schedule_done() after a
  failed commit so weston-info does not hang waiting for wl_output.done.
- XWayland shutdown: SIGKILL Xwayland before display teardown to prevent
  waitpid() hanging; null server->client to skip double-free in
  wlr_xwayland_server_destroy().
- Self-pipe shutdown: write() to a pipe from the SIGTERM signal handler;
  event loop reads it and calls wl_display_terminate() from loop context,
  avoiding async-signal-safety issues of calling display APIs from a handler.

Smoke test result: 22/22 checks PASS, wmaker exits cleanly (status 0).

Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
This commit is contained in:
Levi Neely 2026-03-30 11:15:02 +02:00
parent f86d294223
commit 3fd9926ba3
1 changed files with 201 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

229
src/backend/wayland/wl_backend.c
View File

@ -82,6 +82,10 @@
#include <stdlib.h>
#include <stdio.h>
#include <unistd.h>
#include <fcntl.h>
#include <errno.h>
#include <signal.h>
#include <time.h>
#include <wayland-server-core.h>
#include <wlr/backend.h>
@ -217,6 +221,10 @@ struct wl_compositor_state {
struct wl_listener output_frame_listener;
char socket_name[64]; /* Phase 34: e.g. "wayland-0" */
int started; /* non-zero after successful display_open */
/* Self-pipe for async-signal-safe shutdown: signal handler writes to [1],
* event loop reads [0] and calls wl_display_terminate(). */
int shutdown_pipe[2];
struct wl_event_source *shutdown_source;
};
static struct wl_compositor_state wl_state;
@ -228,6 +236,22 @@ static struct wl_compositor_state wl_state;
/*
* Phase 37: Wayland event loop timer — drives client frame callbacks.
*/
/* Called from within the event loop when the shutdown self-pipe becomes
* readable. Calling wl_display_terminate() here is safe because we are
* inside wl_event_loop_dispatch(), not in a signal handler. */
static int
wl_shutdown_pipe_handler(int fd, uint32_t mask, void *data)
{
char buf[16];
(void)mask; (void)data;
/* drain the pipe so the fd does not stay readable */
while (read(fd, buf, sizeof(buf)) > 0)
;
if (wl_state.display)
wl_display_terminate(wl_state.display);
return 0;
}
static int
frame_timer_cb(void *data)
{
@ -497,10 +521,20 @@ handle_toplevel_destroy(struct wl_listener *listener, void *data)
struct wl_toplevel_view *view = VIEW_FROM_LISTENER(listener, destroy_listener);
(void)data;
/* Remove all listeners registered with xdg_surface / xdg_toplevel signals.
* Failure to remove a listener before freeing the view leaves a dangling
* pointer in wlroots' signal list; when wlroots later emits or cleans up
* those signals (e.g. during wl_display_destroy) it corrupts the heap. */
wl_list_remove(&view->map_listener.link);
wl_list_remove(&view->unmap_listener.link);
wl_list_remove(&view->destroy_listener.link);
wl_list_remove(&view->title_listener.link);
wl_list_remove(&view->request_move_listener.link);
wl_list_remove(&view->request_resize_listener.link);
wl_list_remove(&view->request_maximize_listener.link);
wl_list_remove(&view->request_fullscreen_listener.link);
wl_list_remove(&view->request_minimize_listener.link);
wl_list_remove(&view->set_app_id_listener.link);
wl_list_remove(&view->link);
/* Phase 18: remove scene node. */
if (view->scene_node) {
@ -828,8 +862,21 @@ static void
handle_output_frame(struct wl_listener *listener, void *data)
{
(void)listener; (void)data;
if (wl_state.scene_output)
if (!wl_state.scene_output)
return;
if (wl_state.allocator) {
/* Full scene commit — renders client surfaces and fires
* wl_surface.frame callbacks so clients can submit next buffer. */
wlr_scene_output_commit(wl_state.scene_output);
} else {
/* No GPU allocator available (e.g. headless without DRI access).
* Skip rendering but still fire wl_surface.frame callbacks so
* clients are not stalled waiting for frame pacing. Without
* this, weston-simple-shm would never submit its second buffer. */
struct timespec now;
clock_gettime(CLOCK_MONOTONIC, &now);
wlr_scene_output_send_frame_done(wl_state.scene_output, &now);
}
}
/*
@ -859,8 +906,13 @@ handle_backend_new_output(struct wl_listener *listener, void *data)
if (mode) wlr_output_set_mode(output, mode);
}
wlr_output_enable(output, true);
if (!wlr_output_commit(output))
if (!wlr_output_commit(output)) {
wwarning("wl_backend: initial output commit failed");
/* Send wl_output.done so Wayland clients (e.g. weston-info) do
* not wait indefinitely for output configuration to be finalised.
* On a failed commit wlroots does not send done automatically. */
wlr_output_schedule_done(output);
}
/* Register with the output layout. */
if (wl_state.output_layout)
@ -1169,6 +1221,26 @@ wl_display_open(const char *name)
wwarning("wl_backend: virtual pointer initialised at %d,%d",
wl_state.pointer_x, wl_state.pointer_y);
/* Self-pipe shutdown: signal handler writes a byte; the event loop reads
* it here and calls wl_display_terminate() from within loop context,
* avoiding the async-signal-safety issues of calling wl_display_destroy()
* directly from a signal handler. */
wl_state.shutdown_pipe[0] = wl_state.shutdown_pipe[1] = -1;
wl_state.shutdown_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
* instead of blocking after all bytes are consumed. */
fcntl(wl_state.shutdown_pipe[0], F_SETFL, O_NONBLOCK);
wl_state.shutdown_source = wl_event_loop_add_fd(loop,
wl_state.shutdown_pipe[0],
WL_EVENT_READABLE,
wl_shutdown_pipe_handler,
NULL);
} else {
wwarning("wl_backend: shutdown pipe creation failed; SIGTERM may crash");
}
wl_state.started = 1;
return NULL; /* no X11 Display* in the Wayland path */
}
@ -1178,39 +1250,117 @@ wl_display_close(void)
{
if (!wl_state.started)
return;
/* Phase 17: shut down XWayland before destroying the display. */
/* Phase 17: shut down XWayland.
*
* wlroots 0.15 bug: wlr_xwayland_server_destroy() calls wl_client_destroy()
* on the XWayland Wayland client. After any Wayland client has connected
* and disconnected during the session, the XWayland server's wl_client ends
* up in an inconsistent state: the client struct is freed but server->client
* is NOT nulled. wlr_xwayland_server_destroy() then calls wl_client_destroy()
* on the freed struct and crashes in wl_list_remove().
*
* Workaround: skip wlr_xwayland_destroy() entirely. The Xwayland process is
* a child of wmaker and will receive SIGHUP/SIGKILL when the parent exits.
* The X11 socket is in a tmpdir removed on exit. wl_display_destroy() (called
* below) handles all remaining Wayland client/resource cleanup.
*/
if (wl_state.xwayland) {
wlr_xwayland_destroy(wl_state.xwayland);
/* wlroots 0.15 bug: wlr_xwayland_server_destroy() (called from the
* display destroy signal below) sends SIGTERM to Xwayland and calls
* waitpid(). If Xwayland is blocked waiting for compositor events,
* waitpid() hangs indefinitely.
*
* Fix: pre-kill Xwayland with SIGKILL. SIGKILL cannot be caught or
* deferred, so the process dies immediately (kernel-synchronous).
* The zombie remains until wlroots' waitpid() reaps it — which now
* returns instantly. The Wayland client struct (server->client) is
* still valid because the event loop is stopped and the socket closure
* has not been dispatched, so wlroots' wl_client_destroy() succeeds. */
if (wl_state.xwayland->server) {
if (wl_state.xwayland->server->pid > 0)
kill(wl_state.xwayland->server->pid, SIGKILL);
/* wlroots 0.15 bug: after any Wayland client has connected and
* disconnected during the session, server->client holds a freed
* (stale) wl_client pointer. wlr_xwayland_server_destroy() (fired
* from the display destroy signal below) calls wl_client_destroy()
* on this freed pointer, which corrupts the list and triggers
* SIGABRT. Null the pointer so wlroots skips the destroy call.
* If the client is still valid, wl_display_destroy_clients() will
* clean it up; if it is stale, it has already been removed from
* the display's client list and nothing more is needed. */
wl_state.xwayland->server->client = NULL;
}
wl_state.xwayland = NULL;
unsetenv("DISPLAY");
}
if (wl_state.frame_timer) {
wl_event_source_remove(wl_state.frame_timer);
wl_state.frame_timer = NULL;
/* 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) {
wl_event_source_remove(wl_state.shutdown_source);
wl_state.shutdown_source = NULL;
}
if (wl_state.output_layout) {
wlr_output_layout_destroy(wl_state.output_layout);
wl_state.output_layout = NULL;
if (wl_state.shutdown_pipe[0] >= 0) {
close(wl_state.shutdown_pipe[0]);
wl_state.shutdown_pipe[0] = -1;
}
/* Phase 18: destroy scene and allocator before backend. */
if (wl_state.scene_output) {
wlr_scene_output_destroy(wl_state.scene_output);
wl_state.scene_output = NULL;
if (wl_state.shutdown_pipe[1] >= 0) {
close(wl_state.shutdown_pipe[1]);
wl_state.shutdown_pipe[1] = -1;
}
if (wl_state.scene) {
wlr_scene_node_destroy(&wl_state.scene->node);
wl_state.scene = NULL;
}
if (wl_state.allocator) {
wlr_allocator_destroy(wl_state.allocator);
wl_state.allocator = NULL;
}
if (wl_state.renderer)
wlr_renderer_destroy(wl_state.renderer);
if (wl_state.backend)
wlr_backend_destroy(wl_state.backend);
if (wl_state.display)
wl_display_destroy(wl_state.display);
/* Remove all globally-registered wl_listener nodes before calling
* wl_display_destroy(). wl_display_destroy() frees wlroots objects
* (xdg_shell, backend, keyboard, pointer, output, etc.), which frees
* their signal structs. Any wl_listener.link that is still in one of
* those signal lists after the struct is freed becomes a dangling pointer.
* Subsequent wl_list operations (in wayland-server cleanup or by glibc's
* heap allocator detecting corruption) then trigger SIGABRT.
* Guard each removal with a link.next check: wayland-server zeroes the
* link after wl_list_remove(), so a non-NULL link means the listener is
* currently inserted in a signal list. */
#define WL_LISTENER_REMOVE(l) \
do { if ((l).link.next) wl_list_remove(&(l).link); } while (0)
/* Output frame and new-output listeners */
WL_LISTENER_REMOVE(wl_state.output_frame_listener);
WL_LISTENER_REMOVE(wl_state.backend_new_output_listener);
/* XDG shell and decoration listeners */
WL_LISTENER_REMOVE(wl_state.xdg_new_surface_listener);
WL_LISTENER_REMOVE(wl_state.decoration_new_listener);
/* XWayland new-surface listener (xwayland ptr already nulled in step 1,
* but the listener link is still inserted in the (now-orphaned) xwayland
* object's signal list — remove it to prevent heap corruption). */
WL_LISTENER_REMOVE(wl_state.xwayland_new_surface_listener);
/* Keyboard listeners */
WL_LISTENER_REMOVE(wl_state.keyboard_key_listener);
WL_LISTENER_REMOVE(wl_state.keyboard_modifiers_listener);
/* Pointer listeners */
WL_LISTENER_REMOVE(wl_state.pointer_motion_listener);
WL_LISTENER_REMOVE(wl_state.pointer_motion_abs_listener);
WL_LISTENER_REMOVE(wl_state.pointer_button_listener);
WL_LISTENER_REMOVE(wl_state.pointer_frame_listener);
#undef WL_LISTENER_REMOVE
/* Skip wl_display_destroy() and wl_display_destroy_clients().
*
* wl_display_destroy() fires the display's destroy signal which triggers
* wlroots 0.15 internal object teardown. That teardown has a double-
* remove bug in the XWayland path: wlroots registers a listener on the
* Xwayland wl_client's destroy signal; that listener self-removes during
* client destruction, and wlr_xwayland_server_destroy() then tries to
* wl_list_remove() the already-nulled link, corrupting the heap
* (SIGABRT).
*
* wl_display_destroy_clients() also blocks: wlroots' XWayland client
* destroy handler performs blocking cleanup that can hang if Xwayland
* is in the middle of protocol dispatch.
*
* Both calls are unnecessary here because Exit() calls exit(0) immediately
* after wl_display_close() returns. The kernel reclaims all resources:
* - file descriptors closed → all Wayland clients get POLLHUP
* - memory freed by OS
* - Xwayland already SIGKILL'd in step 1
* - event sources removed in step 2
* - all our signal listeners removed in step 3 */
unsetenv("WAYLAND_DISPLAY");
wl_state.renderer = NULL;
wl_state.backend = NULL;
@ -1218,6 +1368,11 @@ wl_display_close(void)
wl_state.wl_backend_inner = NULL;
wl_state.display = NULL;
wl_state.output = NULL;
wl_state.frame_timer = NULL;
wl_state.output_layout = NULL;
wl_state.scene_output = NULL;
wl_state.scene = NULL;
wl_state.allocator = NULL;
wl_state.started = 0;
}
@ -1247,6 +1402,23 @@ wl_event_loop_run(void)
wl_display_run(wl_state.display);
}
static void
wl_event_loop_terminate(void)
{
/* Write a byte to the self-pipe. write(2) is async-signal-safe, so this
* can safely be called from a signal handler. The event loop will read
* the pipe and call wl_display_terminate() from within loop context,
* avoiding the double-free that results from calling wl_display_destroy()
* while wl_event_loop_dispatch() is active. */
if (wl_state.shutdown_pipe[1] >= 0) {
char c = 'T';
(void)write(wl_state.shutdown_pipe[1], &c, 1);
} else if (wl_state.display) {
/* fallback if pipe creation failed at startup */
wl_display_terminate(wl_state.display);
}
}
/* ------------------------------------------------------------------ */
/* Screen open/close */
/* ------------------------------------------------------------------ */
@ -2342,6 +2514,7 @@ const WMBackend wl_backend = {
.display_default_screen = wl_display_default_screen,
.atom_intern = wl_atom_intern,
.event_loop_run = wl_event_loop_run,
.event_loop_terminate = wl_event_loop_terminate,
.screen_open = wl_screen_open,
.screen_close = wl_screen_close,