From a3f22240c29de746df6492dc331580eb677c942e Mon Sep 17 00:00:00 2001 From: Leandro Ribeiro Date: Sun, 1 Feb 2026 14:45:38 -0300 Subject: [PATCH 1/6] xdg-shell: handle xdg_wm_base being destroyed before its children According to the xdg-shell protocol specification, if the xdg_wm_base object is destroyed while there are still xdg_surface objects associated with it, the compositor must post a protocol error (DEFUNCT_SURFACES) to the client. In this patch we do that. Signed-off-by: Leandro Ribeiro --- libweston/desktop/xdg-shell-v6.c | 21 ++++++++++++++++++++- libweston/desktop/xdg-shell.c | 21 ++++++++++++++++++++- 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/libweston/desktop/xdg-shell-v6.c b/libweston/desktop/xdg-shell-v6.c index 1f11416d1..9b62727d9 100644 --- a/libweston/desktop/xdg-shell-v6.c +++ b/libweston/desktop/xdg-shell-v6.c @@ -1514,8 +1514,27 @@ weston_desktop_xdg_shell_protocol_pong(struct wl_client *wl_client, weston_desktop_client_pong(client, serial); } +static void +weston_desktop_xdg_shell_protocol_destroy(struct wl_client *wl_client, + struct wl_resource *resource) +{ + struct weston_desktop_client *client = + wl_resource_get_user_data(resource); + struct wl_list *surface_list = + weston_desktop_client_get_surface_list(client); + + if (!wl_list_empty(surface_list)) { + wl_resource_post_error(resource, + ZXDG_SHELL_V6_ERROR_DEFUNCT_SURFACES, + "xdg_wm_base being destroyed before child surfaces"); + return; + } + + weston_desktop_destroy_request(wl_client, resource); +} + static const struct zxdg_shell_v6_interface weston_desktop_xdg_shell_implementation = { - .destroy = weston_desktop_destroy_request, + .destroy = weston_desktop_xdg_shell_protocol_destroy, .create_positioner = weston_desktop_xdg_shell_protocol_create_positioner, .get_xdg_surface = weston_desktop_xdg_shell_protocol_get_xdg_surface, .pong = weston_desktop_xdg_shell_protocol_pong, diff --git a/libweston/desktop/xdg-shell.c b/libweston/desktop/xdg-shell.c index cd784da24..818345569 100644 --- a/libweston/desktop/xdg-shell.c +++ b/libweston/desktop/xdg-shell.c @@ -2102,8 +2102,27 @@ weston_desktop_xdg_shell_protocol_pong(struct wl_client *wl_client, weston_desktop_client_pong(client, serial); } +static void +weston_desktop_xdg_shell_protocol_destroy(struct wl_client *wl_client, + struct wl_resource *resource) +{ + struct weston_desktop_client *client = + wl_resource_get_user_data(resource); + struct wl_list *surface_list = + weston_desktop_client_get_surface_list(client); + + if (!wl_list_empty(surface_list)) { + wl_resource_post_error(resource, + XDG_WM_BASE_ERROR_DEFUNCT_SURFACES, + "xdg_wm_base being destroyed before child surfaces"); + return; + } + + weston_desktop_destroy_request(wl_client, resource); +} + static const struct xdg_wm_base_interface weston_desktop_xdg_shell_implementation = { - .destroy = weston_desktop_destroy_request, + .destroy = weston_desktop_xdg_shell_protocol_destroy, .create_positioner = weston_desktop_xdg_shell_protocol_create_positioner, .get_xdg_surface = weston_desktop_xdg_shell_protocol_get_xdg_surface, .pong = weston_desktop_xdg_shell_protocol_pong, From 385647d3576c812e8d2decb4d0551b006b7f4130 Mon Sep 17 00:00:00 2001 From: Leandro Ribeiro Date: Mon, 2 Feb 2026 13:34:12 -0300 Subject: [PATCH 2/6] xdg-shell: avoid leaving dangling pointers on resource creation failure Currently if weston_desktop_surface_add_resource() fails to create a wl_resource, it disconnects the client and destroys the desktop surface that we pass. It is odd to do that, callers should be responsible for destroying the desktop surface when it is reasonable to do so. This is dangerous and can leave dangling pointers. Besides that, in many cases callers should not even destroy the desktop surface, as they are not the owners of it. When we are giving the role of toplevel/popup to a xdg_surface and we fail to do so, client gets disconnected and the base desktop surface will get destroyed by the destructor of the xdg_surface resource. This commit fixes these issues, bringing a more consistent behavior. Signed-off-by: Leandro Ribeiro --- libweston/desktop/surface.c | 1 - libweston/desktop/xdg-shell-v6.c | 7 ++++++- libweston/desktop/xdg-shell.c | 5 ++++- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/libweston/desktop/surface.c b/libweston/desktop/surface.c index d6d0188f9..c93f16113 100644 --- a/libweston/desktop/surface.c +++ b/libweston/desktop/surface.c @@ -330,7 +330,6 @@ weston_desktop_surface_add_resource(struct weston_desktop_surface *surface, id); if (resource == NULL) { wl_client_post_no_memory(wl_client); - weston_desktop_surface_destroy(surface); return NULL; } if (destroy == NULL) diff --git a/libweston/desktop/xdg-shell-v6.c b/libweston/desktop/xdg-shell-v6.c index 9b62727d9..b3473c2a4 100644 --- a/libweston/desktop/xdg-shell-v6.c +++ b/libweston/desktop/xdg-shell-v6.c @@ -1492,13 +1492,18 @@ weston_desktop_xdg_shell_protocol_get_xdg_surface(struct wl_client *wl_client, &zxdg_surface_v6_interface, &weston_desktop_xdg_surface_implementation, id, weston_desktop_xdg_surface_resource_destroy); - if (surface->resource == NULL) + if (surface->resource == NULL) { + weston_desktop_surface_destroy(surface->desktop_surface); + free(surface); return; + } if (weston_surface_has_content(wsurface)) { wl_resource_post_error(surface->resource, ZXDG_SURFACE_V6_ERROR_UNCONFIGURED_BUFFER, "xdg_surface must not have a buffer at creation"); + weston_desktop_surface_destroy(surface->desktop_surface); + free(surface); return; } } diff --git a/libweston/desktop/xdg-shell.c b/libweston/desktop/xdg-shell.c index 818345569..3c025d3f6 100644 --- a/libweston/desktop/xdg-shell.c +++ b/libweston/desktop/xdg-shell.c @@ -2087,8 +2087,11 @@ weston_desktop_xdg_shell_protocol_get_xdg_surface(struct wl_client *wl_client, &xdg_surface_interface, &weston_desktop_xdg_surface_implementation, id, weston_desktop_xdg_surface_resource_destroy); - if (surface->resource == NULL) + if (surface->resource == NULL) { + weston_desktop_surface_destroy(surface->desktop_surface); + free(surface); return; + } } static void From af5419b87c11a71fb6579b07133cdd4ec2c2c51b Mon Sep 17 00:00:00 2001 From: Leandro Ribeiro Date: Mon, 2 Feb 2026 11:40:16 -0300 Subject: [PATCH 3/6] xdg-shell: ensure that client resource is not NULL before use Functions that implement the xdg_surface, toplevel and popup interfaces may depend on struct weston_desktop_client::resource. In normal circumstances, this resource can't be NULL. The xdg protocol extension forbids destroying xdg_wm_base before destroying all its child surfaces. If that happens, we disconnect the client with protocol error DEFUNCT_SURFACES (see commit "xdg-shell: handle xdg_wm_base being destroyed before its children"). But during client teardown resources are destroyed in arbitrary order, so it is possible that client resource becomes NULL before its surfaces are destroyed. This commit adds checks to avoid using NULL client resource, in order to avoid crashes. It is safe to silently do nothing in these cases, as the client is being destroyed anyway. Signed-off-by: Leandro Ribeiro --- libweston/desktop/surface.c | 3 ++ libweston/desktop/xdg-shell-v6.c | 37 ++++++++++------ libweston/desktop/xdg-shell.c | 74 +++++++++++++++++--------------- 3 files changed, 65 insertions(+), 49 deletions(-) diff --git a/libweston/desktop/surface.c b/libweston/desktop/surface.c index c93f16113..0fa5da487 100644 --- a/libweston/desktop/surface.c +++ b/libweston/desktop/surface.c @@ -324,6 +324,9 @@ weston_desktop_surface_add_resource(struct weston_desktop_surface *surface, weston_desktop_client_get_client(surface->client); struct wl_resource *resource; + if (!client_resource) + return NULL; + resource = wl_resource_create(wl_client, interface, wl_resource_get_version(client_resource), diff --git a/libweston/desktop/xdg-shell-v6.c b/libweston/desktop/xdg-shell-v6.c index b3473c2a4..1f48a55a3 100644 --- a/libweston/desktop/xdg-shell-v6.c +++ b/libweston/desktop/xdg-shell-v6.c @@ -674,9 +674,10 @@ weston_desktop_xdg_toplevel_committed(struct weston_desktop_xdg_toplevel *toplev struct wl_resource *client_resource = weston_desktop_client_get_resource(client); - wl_resource_post_error(client_resource, - ZXDG_SHELL_V6_ERROR_INVALID_SURFACE_STATE, - "xdg_surface buffer does not match the configured state"); + if (client_resource) + wl_resource_post_error(client_resource, + ZXDG_SHELL_V6_ERROR_INVALID_SURFACE_STATE, + "xdg_surface buffer does not match the configured state"); return; } @@ -856,9 +857,10 @@ weston_desktop_xdg_popup_protocol_grab(struct wl_client *wl_client, struct wl_resource *client_resource = weston_desktop_client_get_resource(client); - wl_resource_post_error(client_resource, - ZXDG_SHELL_V6_ERROR_NOT_THE_TOPMOST_POPUP, - "xdg_popup was not created on the topmost popup"); + if (client_resource) + wl_resource_post_error(client_resource, + ZXDG_SHELL_V6_ERROR_NOT_THE_TOPMOST_POPUP, + "xdg_popup was not created on the topmost popup"); return; } @@ -934,9 +936,10 @@ weston_desktop_xdg_popup_destroy(struct weston_desktop_xdg_popup *popup) struct wl_resource *client_resource = weston_desktop_client_get_resource(client); - wl_resource_post_error(client_resource, - ZXDG_SHELL_V6_ERROR_NOT_THE_TOPMOST_POPUP, - "xdg_popup was destroyed while it was not the topmost popup."); + if (client_resource) + wl_resource_post_error(client_resource, + ZXDG_SHELL_V6_ERROR_NOT_THE_TOPMOST_POPUP, + "xdg_popup was destroyed while it was not the topmost popup."); } weston_desktop_surface_popup_ungrab(popup->base.desktop_surface, @@ -1257,9 +1260,11 @@ weston_desktop_xdg_surface_protocol_ack_configure(struct wl_client *wl_client, weston_desktop_surface_get_client(dsurface); struct wl_resource *client_resource = weston_desktop_client_get_resource(client); - wl_resource_post_error(client_resource, - ZXDG_SHELL_V6_ERROR_INVALID_SURFACE_STATE, - "Wrong configure serial: %u", serial); + + if (client_resource) + wl_resource_post_error(client_resource, + ZXDG_SHELL_V6_ERROR_INVALID_SURFACE_STATE, + "Wrong configure serial: %u", serial); return; } @@ -1286,9 +1291,13 @@ weston_desktop_xdg_surface_ping(struct weston_desktop_surface *dsurface, { struct weston_desktop_client *client = weston_desktop_surface_get_client(dsurface); + struct wl_resource *client_resource = + weston_desktop_client_get_resource(client); - zxdg_shell_v6_send_ping(weston_desktop_client_get_resource(client), - serial); + if (!client_resource) + return; + + zxdg_shell_v6_send_ping(client_resource, serial); } static void diff --git a/libweston/desktop/xdg-shell.c b/libweston/desktop/xdg-shell.c index 3c025d3f6..8ca5dd6f1 100644 --- a/libweston/desktop/xdg-shell.c +++ b/libweston/desktop/xdg-shell.c @@ -1082,6 +1082,10 @@ weston_desktop_xdg_toplevel_committed(struct weston_desktop_xdg_toplevel *toplev { struct weston_surface *wsurface = weston_desktop_surface_get_surface(toplevel->base.desktop_surface); + struct weston_desktop_client *client = + weston_desktop_surface_get_client(toplevel->base.desktop_surface); + struct wl_resource *client_resource = + weston_desktop_client_get_resource(client); if (!weston_surface_has_content(wsurface) && !toplevel->added) { weston_desktop_xdg_toplevel_ensure_added(toplevel); @@ -1101,36 +1105,28 @@ weston_desktop_xdg_toplevel_committed(struct weston_desktop_xdg_toplevel *toplev if (toplevel->next.state.maximized && (toplevel->next.size.width != geometry.width || toplevel->next.size.height != geometry.height)) { - struct weston_desktop_client *client = - weston_desktop_surface_get_client(toplevel->base.desktop_surface); - struct wl_resource *client_resource = - weston_desktop_client_get_resource(client); - - wl_resource_post_error(client_resource, - XDG_WM_BASE_ERROR_INVALID_SURFACE_STATE, - "xdg_surface geometry (%" PRIi32 " x %" PRIi32 ") " - "does not match the configured maximized state (%" PRIi32 " x %" PRIi32 ")", - geometry.width, geometry.height, - toplevel->next.size.width, - toplevel->next.size.height); + if (client_resource) + wl_resource_post_error(client_resource, + XDG_WM_BASE_ERROR_INVALID_SURFACE_STATE, + "xdg_surface geometry (%" PRIi32 " x %" PRIi32 ") " + "does not match the configured maximized state (%" PRIi32 " x %" PRIi32 ")", + geometry.width, geometry.height, + toplevel->next.size.width, + toplevel->next.size.height); return; } if (toplevel->next.state.fullscreen && (toplevel->next.size.width < geometry.width || toplevel->next.size.height < geometry.height)) { - struct weston_desktop_client *client = - weston_desktop_surface_get_client(toplevel->base.desktop_surface); - struct wl_resource *client_resource = - weston_desktop_client_get_resource(client); - - wl_resource_post_error(client_resource, - XDG_WM_BASE_ERROR_INVALID_SURFACE_STATE, - "xdg_surface geometry (%" PRIi32 " x %" PRIi32 ") " - "is larger than the configured fullscreen state (%" PRIi32 " x %" PRIi32 ")", - geometry.width, geometry.height, - toplevel->next.size.width, - toplevel->next.size.height); + if (client_resource) + wl_resource_post_error(client_resource, + XDG_WM_BASE_ERROR_INVALID_SURFACE_STATE, + "xdg_surface geometry (%" PRIi32 " x %" PRIi32 ") " + "is larger than the configured fullscreen state (%" PRIi32 " x %" PRIi32 ")", + geometry.width, geometry.height, + toplevel->next.size.width, + toplevel->next.size.height); return; } @@ -1299,9 +1295,10 @@ weston_desktop_xdg_popup_protocol_grab(struct wl_client *wl_client, struct wl_resource *client_resource = weston_desktop_client_get_resource(client); - wl_resource_post_error(client_resource, - XDG_WM_BASE_ERROR_NOT_THE_TOPMOST_POPUP, - "xdg_popup was not created on the topmost popup"); + if (client_resource) + wl_resource_post_error(client_resource, + XDG_WM_BASE_ERROR_NOT_THE_TOPMOST_POPUP, + "xdg_popup was not created on the topmost popup"); return; } @@ -1437,9 +1434,10 @@ weston_desktop_xdg_popup_destroy(struct weston_desktop_xdg_popup *popup) struct wl_resource *client_resource = weston_desktop_client_get_resource(client); - wl_resource_post_error(client_resource, - XDG_WM_BASE_ERROR_NOT_THE_TOPMOST_POPUP, - "xdg_popup was destroyed while it was not the topmost popup."); + if (client_resource) + wl_resource_post_error(client_resource, + XDG_WM_BASE_ERROR_NOT_THE_TOPMOST_POPUP, + "xdg_popup was destroyed while it was not the topmost popup."); } weston_desktop_surface_popup_ungrab(popup->base.desktop_surface, @@ -1830,9 +1828,11 @@ weston_desktop_xdg_surface_protocol_ack_configure(struct wl_client *wl_client, weston_desktop_surface_get_client(dsurface); struct wl_resource *client_resource = weston_desktop_client_get_resource(client); - wl_resource_post_error(client_resource, - XDG_WM_BASE_ERROR_INVALID_SURFACE_STATE, - "Wrong configure serial: %u", serial); + + if (client_resource) + wl_resource_post_error(client_resource, + XDG_WM_BASE_ERROR_INVALID_SURFACE_STATE, + "Wrong configure serial: %u", serial); return; } @@ -1859,9 +1859,13 @@ weston_desktop_xdg_surface_ping(struct weston_desktop_surface *dsurface, { struct weston_desktop_client *client = weston_desktop_surface_get_client(dsurface); + struct wl_resource *client_resource = + weston_desktop_client_get_resource(client); - xdg_wm_base_send_ping(weston_desktop_client_get_resource(client), - serial); + if (!client_resource) + return; + + xdg_wm_base_send_ping(client_resource, serial); } static void From fda632fd0a705bc75593367715ef84295a7c61f9 Mon Sep 17 00:00:00 2001 From: Leandro Ribeiro Date: Mon, 2 Feb 2026 22:28:15 -0300 Subject: [PATCH 4/6] tests: use SHELL_TEST_DESKTOP in xdg-shell-initial-commit There's no need to use the desktop-shell if the purpose of these tests is to test our xdg-shell implementation. In the next commits it will be important to have a simpler shell to work with, as we'll introduce additional tests for xdg-shell that trigger leaks that are hard to fix with the desktop-shell. So let's change this test file to use SHELL_TEST_DESKTOP. Signed-off-by: Leandro Ribeiro --- tests/xdg-shell-intial-commit.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/xdg-shell-intial-commit.c b/tests/xdg-shell-intial-commit.c index ef203bb56..f1c4a0f0a 100644 --- a/tests/xdg-shell-intial-commit.c +++ b/tests/xdg-shell-intial-commit.c @@ -48,7 +48,7 @@ fixture_setup(struct weston_test_harness *harness) setup.renderer = WESTON_RENDERER_PIXMAN; setup.width = 320; setup.height = 240; - setup.shell = SHELL_DESKTOP; + setup.shell = SHELL_TEST_DESKTOP; setup.logging_scopes = "proto,log,test-harness-plugin"; setup.refresh = HIGHEST_OUTPUT_REFRESH; From 96d19cbe03345b1edf73873677e21312db3a250e Mon Sep 17 00:00:00 2001 From: Leandro Ribeiro Date: Mon, 2 Feb 2026 09:42:44 -0300 Subject: [PATCH 5/6] tests: rename xdg-shell-initial-commit to xdg-shell-test In the next commit we add more tests to this, so let's rename. Signed-off-by: Leandro Ribeiro --- tests/meson.build | 4 ++-- tests/{xdg-shell-intial-commit.c => xdg-shell-test.c} | 0 2 files changed, 2 insertions(+), 2 deletions(-) rename tests/{xdg-shell-intial-commit.c => xdg-shell-test.c} (100%) diff --git a/tests/meson.build b/tests/meson.build index faf272dd2..e3645ef9b 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -372,9 +372,9 @@ if get_option('shell-desktop') ], }, { - 'name': 'xdg-shell-initial-commit', + 'name': 'xdg-shell-test', 'sources': [ - 'xdg-shell-intial-commit.c', + 'xdg-shell-test.c', xdg_shell_client_protocol_h, xdg_shell_protocol_c, ], diff --git a/tests/xdg-shell-intial-commit.c b/tests/xdg-shell-test.c similarity index 100% rename from tests/xdg-shell-intial-commit.c rename to tests/xdg-shell-test.c From 9124524cd5e33a75b5d71c11f120e43e7852ec95 Mon Sep 17 00:00:00 2001 From: Leandro Ribeiro Date: Mon, 2 Feb 2026 10:31:16 -0300 Subject: [PATCH 6/6] tests: add xdg-shell defunct surfaces test Since commit "xdg-shell: handle xdg_wm_base being destroyed before its children", we raise a protocol error DEFUNCT_SURFACES for misbehaved clients. This adds a test to ensure that DEFUNCT_SURFACES is being posted for such clients. Signed-off-by: Leandro Ribeiro --- tests/xdg-shell-test.c | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/tests/xdg-shell-test.c b/tests/xdg-shell-test.c index f1c4a0f0a..69278c2d2 100644 --- a/tests/xdg-shell-test.c +++ b/tests/xdg-shell-test.c @@ -182,3 +182,26 @@ TEST(initial_commit_without_a_buffer_subsurface) return RESULT_OK; } + +TEST(defunct_surfaces) +{ + struct xdg_client *xdg_client = create_xdg_client(); + struct xdg_surface_data *xdg_surface = create_xdg_surface(xdg_client); + + xdg_surface_make_toplevel(xdg_surface, "weston.test", "xdg-test"); + xdg_surface_wait_configure(xdg_surface); + + xdg_surface_commit_solid(xdg_surface, 255, 128, 255); + wl_display_roundtrip(xdg_client->client->wl_display); + + /* Destroy xdg_wm_base without destroying its children. */ + xdg_wm_base_destroy(xdg_client->xdg_wm_base); + expect_protocol_error(xdg_client->client, NULL, XDG_WM_BASE_ERROR_DEFUNCT_SURFACES); + + destroy_xdg_surface(xdg_surface); + + client_destroy(xdg_client->client); + free(xdg_client); + + return RESULT_OK; +}