diff --git a/src/backend/wayland/wl_backend.c b/src/backend/wayland/wl_backend.c index a42ddf3..a99f638 100644 --- a/src/backend/wayland/wl_backend.c +++ b/src/backend/wayland/wl_backend.c @@ -82,6 +82,10 @@ #include #include #include +#include +#include +#include +#include #include #include @@ -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,