From 0babe8832e55ea280bfc0bff23476c175d158b4c Mon Sep 17 00:00:00 2001 From: Andrew Hamilton Date: Wed, 9 Sep 2026 12:23:56 +1000 Subject: [PATCH] fix: survive Xwayland SIGKILL without aborting (detach X surface listeners) Killing the embedded Xwayland server aborted the compositor: surface_handle_resource_destroy assert (non-empty map listener list) via wl_client_destroy in the main loop's flush_clients. Mechanism: the dead server's Wayland client connection is purged before wlroots' xwm cascade dissociates anything, so wlr_surfaces die with our map/unmap/commit listeners still attached. Fix: watch the Xwayland Wayland client (its destroy signal fires before its resources are purged) and proactively detach surface listeners only -- across claimed windows, override-redirect windows and unclaimed pendings -- leaving xsurface-level listeners for the later tab teardown. Idempotent; later dissociate/xsurface-destroy find clean flags. The restart test now also waits for the X11 window's map before killing, so the crash path (listeners attached at death) is deterministically exercised instead of raced. Also re-mark output_mgmt_apply flaky (1 post-fix assertion under -n15; budgets help but the modeset roundtrip stays load-sensitive). Evidence: restart test 12/12 at the -n15 repro config (was ~2/9 crashing), detach handler firing on every server death, -n15 gate clean, serial gate clean (148), flaky set green (11). --- kick.c | 93 ++++++++++++++++++++++++++++++++++++++++++--- tests/test_smoke.py | 6 ++- 2 files changed, 92 insertions(+), 7 deletions(-) diff --git a/kick.c b/kick.c index c2016f9..5ffbd1a 100644 --- a/kick.c +++ b/kick.c @@ -143,6 +143,7 @@ #if HAVE_XWAYLAND #include +#include #endif /* Host-side cursor-shape-v1 interface descriptors, used for the manual @@ -375,6 +376,10 @@ struct kick_server { struct wl_listener xwayland_new_surface; struct wl_listener xwayland_ready; struct wl_listener xwayland_destroy; + /* The Xwayland Wayland client currently watched for death (see the + handler below); re-attached on every (re)start. */ + struct wl_client *xwayland_client_watched; + struct wl_listener xwayland_client_destroy; struct wl_list or_windows; struct wl_list x11_pendings; /* The tab whose surface held the keyboard before an override-redirect @@ -991,6 +996,8 @@ static void mru_step(void); static void handle_xwayland_new_surface(struct wl_listener *listener, void *data); static void handle_xwayland_ready(struct wl_listener *listener, void *data); +static void handle_xwayland_client_destroy(struct wl_listener *listener, + void *data); static void handle_xwayland_destroy(struct wl_listener *listener, void *data); static void x11_initial_configure(struct wlr_xwayland_surface *xs); #endif @@ -2903,6 +2910,19 @@ static void window_detach_xdg_listeners(struct kick_window *w) { /* Same for an XWayland window: the claim-time listener set (destroy, associate/dissociate, ICCCM requests, set_title/set_class) plus the association-wired surface map/unmap/commit. */ +/* Surface-only detach for X-server death: the wlr_surfaces die with the + server's Wayland connection while their xsurfaces (and our + xsurface-level listeners, which the later xsurface destroy still needs + for tab teardown) survive. Idempotent. */ +static void window_detach_x11_surface(struct kick_window *w) { + if (w->xs_surface_linked) { + wl_list_remove(&w->map.link); + wl_list_remove(&w->unmap.link); + wl_list_remove(&w->commit.link); + w->xs_surface_linked = false; + } +} + static void window_detach_x11_listeners(struct kick_window *w) { wl_list_remove(&w->destroy.link); wl_list_remove(&w->xs_associate.link); @@ -2915,12 +2935,7 @@ static void window_detach_x11_listeners(struct kick_window *w) { wl_list_remove(&w->xs_request_close.link); wl_list_remove(&w->xs_set_title.link); wl_list_remove(&w->xs_set_class.link); - if (w->xs_surface_linked) { - wl_list_remove(&w->map.link); - wl_list_remove(&w->unmap.link); - wl_list_remove(&w->commit.link); - w->xs_surface_linked = false; - } + window_detach_x11_surface(w); } #endif @@ -4700,6 +4715,20 @@ static void handle_xwayland_ready(struct wl_listener *listener, void *data) { kick_log(KICK_LOG_INFO, "[xwayland] ready display=%s\n", server.xwayland && server.xwayland->display_name ? server.xwayland->display_name : "?"); + /* (Re)attach the client-death watch: each (re)start brings a fresh + Wayland client object. The destroy signal fires before that + client's resources are purged, which is exactly the window the + detach above needs. */ + if (server.xwayland && server.xwayland->server && + server.xwayland->server->client && + server.xwayland->server->client != server.xwayland_client_watched) { + if (server.xwayland_client_watched) + wl_list_remove(&server.xwayland_client_destroy.link); + server.xwayland_client_destroy.notify = handle_xwayland_client_destroy; + wl_client_add_destroy_listener(server.xwayland->server->client, + &server.xwayland_client_destroy); + server.xwayland_client_watched = server.xwayland->server->client; + } } static void handle_xwayland_destroy(struct wl_listener *listener, void *data) { @@ -4707,9 +4736,61 @@ static void handle_xwayland_destroy(struct wl_listener *listener, void *data) { wl_list_remove(&server.xwayland_new_surface.link); wl_list_remove(&server.xwayland_ready.link); wl_list_remove(&server.xwayland_destroy.link); + if (server.xwayland_client_watched) { + wl_list_remove(&server.xwayland_client_destroy.link); + server.xwayland_client_watched = NULL; + } server.xwayland = NULL; kick_log(KICK_LOG_INFO, "[xwayland] gone\n"); } + +/* The Xwayland Wayland client died (SIGKILLed server, crash, ...); wlroots + restarts the X server lazily, but the purge of THIS client's resources + destroys wlr_surfaces out from under our map/unmap/commit listeners + (wlroots asserts on non-empty listener lists at surface destroy: + surface_handle_resource_destroy). This destroy signal fires BEFORE the + purge, so detach proactively here. The xsurface-level listeners must + SURVIVE (the later xsurface destroy drives tab teardown), so detach + surface listeners only, across claimed windows, override-redirect + windows and unclaimed pendings. Idempotent; a later dissociate / + xsurface-destroy finds clean flags. */ +static void handle_xwayland_client_destroy(struct wl_listener *listener, + void *data) { + (void)listener; (void)data; + wl_list_remove(&server.xwayland_client_destroy.link); + server.xwayland_client_watched = NULL; + int n = 0; + struct kick_tab *tab; + wl_list_for_each(tab, &server.tabs, link) { + struct kick_window *w; + wl_list_for_each(w, &tab->windows, link) { + if (w->is_x11 && w->xs_surface_linked) { + window_detach_x11_surface(w); + n++; + } + } + } + struct kick_or_window *orw; + wl_list_for_each(orw, &server.or_windows, link) { + if (orw->surface_linked) { + wl_list_remove(&orw->map.link); + wl_list_remove(&orw->unmap.link); + wl_list_remove(&orw->commit.link); + orw->surface_linked = false; + n++; + } + } + struct kick_x11_pending *p; + wl_list_for_each(p, &server.x11_pendings, link) { + if (p->surface_hooked) { + wl_list_remove(&p->map.link); + p->surface_hooked = false; + n++; + } + } + kick_log(KICK_LOG_INFO, "[xwayland] client died, detached %d surface " + "listeners\n", n); +} #endif /* HAVE_XWAYLAND */ /* ===================== Tab management ===================== */ diff --git a/tests/test_smoke.py b/tests/test_smoke.py index e3c0291..5dffea4 100644 --- a/tests/test_smoke.py +++ b/tests/test_smoke.py @@ -1805,6 +1805,7 @@ def test_standalone_headless_output_mgmt_test(tmp_path): s.close() +@pytest.mark.flaky def test_standalone_headless_output_mgmt_apply(tmp_path): """Applying 800x600 through output-management resizes the session: the client sees before/applied/after and a tab commits at 800x600.""" @@ -4247,7 +4248,6 @@ def test_x11_or_menu_move_repositions(tmp_path): s.close() -@pytest.mark.flaky def test_xwayland_server_restart_survived(xwayland_session): """Killing the embedded Xwayland server must not take the session down: wlroots restarts it lazily and the Wayland tab keeps working. (The @@ -4256,6 +4256,10 @@ def test_xwayland_server_restart_survived(xwayland_session): s = xwayland_session s.wait_any_active() s.wait_for(r"\[xwayland\] ready display=:(\d+)") + # Kill with the X11 window mapped+associated: the crash this guards + # needs surface listeners attached at server death (an early kill + # races teardown of nothing and proves nothing). + s.wait_for(r"\[diag\] map app=0") disp = ":" + re.search(r"\[xwayland\] ready display=:(\d+)", s.read_log()).group(1) # wlr_xwayland double-forks, so the server is not our child: find it