From 7dde2b66d681f40ca8ad7cb7facbad3772f9fa8b Mon Sep 17 00:00:00 2001 From: Kirill Primak Date: Sat, 1 Jan 2022 14:04:53 +0300 Subject: [PATCH] scene/subsurface_tree: fix handling subsurface destruction This commit renames map/unmap listeners to clarify that they handle subsurface events, and ensures the node is always destroyed before the subsurface. Without this patch, wl_list_remove() would operate on listener links in already freed memory. glibc is usually lenient to bugs like this, but musl isn't. (cherry picked from commit 83ab5055fd36bd0f8a0106257e45d8ed303636d8) --- types/scene/subsurface_tree.c | 65 +++++++++++++++++++++-------------- 1 file changed, 39 insertions(+), 26 deletions(-) diff --git a/types/scene/subsurface_tree.c b/types/scene/subsurface_tree.c index 3f22ee3d..bb3c7ff7 100644 --- a/types/scene/subsurface_tree.c +++ b/types/scene/subsurface_tree.c @@ -13,15 +13,20 @@ struct wlr_scene_subsurface_tree { struct wlr_surface *surface; struct wlr_scene_surface *scene_surface; - struct wlr_scene_subsurface_tree *parent; // NULL for the top-level surface - struct wlr_addon surface_addon; // only set if there's a parent - struct wl_listener tree_destroy; struct wl_listener surface_destroy; struct wl_listener surface_commit; - struct wl_listener surface_map; - struct wl_listener surface_unmap; struct wl_listener surface_new_subsurface; + + struct wlr_scene_subsurface_tree *parent; // NULL for the top-level surface + + // Only valid if the surface is a sub-surface + + struct wlr_addon surface_addon; + + struct wl_listener subsurface_destroy; + struct wl_listener subsurface_map; + struct wl_listener subsurface_unmap; }; static void subsurface_tree_handle_tree_destroy(struct wl_listener *listener, @@ -31,23 +36,17 @@ static void subsurface_tree_handle_tree_destroy(struct wl_listener *listener, // tree and scene_surface will be cleaned up by scene_node_finish if (subsurface_tree->parent) { wlr_addon_finish(&subsurface_tree->surface_addon); + wl_list_remove(&subsurface_tree->subsurface_destroy.link); + wl_list_remove(&subsurface_tree->subsurface_map.link); + wl_list_remove(&subsurface_tree->subsurface_unmap.link); } wl_list_remove(&subsurface_tree->tree_destroy.link); wl_list_remove(&subsurface_tree->surface_destroy.link); wl_list_remove(&subsurface_tree->surface_commit.link); - wl_list_remove(&subsurface_tree->surface_map.link); - wl_list_remove(&subsurface_tree->surface_unmap.link); wl_list_remove(&subsurface_tree->surface_new_subsurface.link); free(subsurface_tree); } -static void subsurface_tree_handle_surface_destroy(struct wl_listener *listener, - void *data) { - struct wlr_scene_subsurface_tree *subsurface_tree = - wl_container_of(listener, subsurface_tree, surface_destroy); - wlr_scene_node_destroy(&subsurface_tree->tree->node); -} - static const struct wlr_addon_interface subsurface_tree_addon_impl; static struct wlr_scene_subsurface_tree *subsurface_tree_from_subsurface( @@ -97,6 +96,13 @@ static void subsurface_tree_reconfigure( } } +static void subsurface_tree_handle_surface_destroy(struct wl_listener *listener, + void *data) { + struct wlr_scene_subsurface_tree *subsurface_tree = + wl_container_of(listener, subsurface_tree, surface_destroy); + wlr_scene_node_destroy(&subsurface_tree->tree->node); +} + static void subsurface_tree_handle_surface_commit(struct wl_listener *listener, void *data) { struct wlr_scene_subsurface_tree *subsurface_tree = @@ -106,18 +112,25 @@ static void subsurface_tree_handle_surface_commit(struct wl_listener *listener, subsurface_tree_reconfigure(subsurface_tree); } -static void subsurface_tree_handle_surface_map(struct wl_listener *listener, +static void subsurface_tree_handle_subsurface_destroy(struct wl_listener *listener, void *data) { struct wlr_scene_subsurface_tree *subsurface_tree = - wl_container_of(listener, subsurface_tree, surface_map); + wl_container_of(listener, subsurface_tree, subsurface_destroy); + wlr_scene_node_destroy(&subsurface_tree->tree->node); +} + +static void subsurface_tree_handle_subsurface_map(struct wl_listener *listener, + void *data) { + struct wlr_scene_subsurface_tree *subsurface_tree = + wl_container_of(listener, subsurface_tree, subsurface_map); wlr_scene_node_set_enabled(&subsurface_tree->tree->node, true); } -static void subsurface_tree_handle_surface_unmap(struct wl_listener *listener, +static void subsurface_tree_handle_subsurface_unmap(struct wl_listener *listener, void *data) { struct wlr_scene_subsurface_tree *subsurface_tree = - wl_container_of(listener, subsurface_tree, surface_unmap); + wl_container_of(listener, subsurface_tree, subsurface_unmap); wlr_scene_node_set_enabled(&subsurface_tree->tree->node, false); } @@ -151,8 +164,14 @@ static bool subsurface_tree_create_subsurface( wlr_addon_init(&child->surface_addon, &subsurface->surface->addons, parent, &subsurface_tree_addon_impl); - wl_signal_add(&subsurface->events.map, &child->surface_map); - wl_signal_add(&subsurface->events.unmap, &child->surface_unmap); + child->subsurface_destroy.notify = subsurface_tree_handle_subsurface_destroy; + wl_signal_add(&subsurface->events.destroy, &child->subsurface_destroy); + + child->subsurface_map.notify = subsurface_tree_handle_subsurface_map; + wl_signal_add(&subsurface->events.map, &child->subsurface_map); + + child->subsurface_unmap.notify = subsurface_tree_handle_subsurface_unmap; + wl_signal_add(&subsurface->events.unmap, &child->subsurface_unmap); return true; } @@ -214,12 +233,6 @@ static struct wlr_scene_subsurface_tree *scene_surface_tree_create( subsurface_tree->surface_commit.notify = subsurface_tree_handle_surface_commit; wl_signal_add(&surface->events.commit, &subsurface_tree->surface_commit); - subsurface_tree->surface_map.notify = subsurface_tree_handle_surface_map; - wl_list_init(&subsurface_tree->surface_map.link); - - subsurface_tree->surface_unmap.notify = subsurface_tree_handle_surface_unmap; - wl_list_init(&subsurface_tree->surface_unmap.link); - subsurface_tree->surface_new_subsurface.notify = subsurface_tree_handle_surface_new_subsurface; wl_signal_add(&surface->events.new_subsurface,