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).
This commit is contained in:
Andrew Hamilton 2026-09-09 12:23:56 +10:00
parent dcd6552533
commit 0babe8832e
2 changed files with 92 additions and 7 deletions

93
kick.c
View file

@ -143,6 +143,7 @@
#if HAVE_XWAYLAND
#include <wlr/xwayland/xwayland.h>
#include <wlr/xwayland/server.h>
#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 ===================== */