tree/container: Remove child from all lists

When a container is detached, we need to remove it from any lists it may
be part of. We use container_get_siblings to obtain the relevant list,
find our entry and remove it.

If the container is in a later list than the one returned by
container_get_siblings, or is in multiple lists for some reason,
container_detach will fail to remove the container, leaving a dangling
pointer when the container is freed.

Instead of calling container_get_siblings, check and remove the
container from all lists.

1 files changed, 40 insertions, 19 deletions

diff --git a/sway/tree/container.c b/sway/tree/container.c
index 2767bc23..af68e22f 100644
--- a/sway/tree/container.c
+++ b/sway/tree/container.c
@@ -500,17 +500,19 @@ void container_update_title_bar(struct sway_container *con) {
 	container_arrange_title_bar(con);
 }
 
+static void container_remove_from_siblings(struct sway_container *child, bool pending);
+
 void container_destroy(struct sway_container *con) {
 	if (con->view) {
 		ipc_event_window(con, "close");
 	}
 	// The workspace must have the fullscreen pointer cleared so that the
 	// seat code can find an appropriate new focus.
-	if (con->pending.fullscreen_mode == FULLSCREEN_WORKSPACE && con->pending.workspace) {
+	if (con->pending.workspace && con->pending.workspace->fullscreen == con) {
 		con->pending.workspace->fullscreen = NULL;
 	}
-	if (con->scratchpad && con->pending.fullscreen_mode == FULLSCREEN_GLOBAL) {
-		container_fullscreen_disable(con);
+	if (con->current.workspace && con->current.workspace->fullscreen == con) {
+		con->current.workspace->fullscreen = NULL;
 	}
 
 	wl_signal_emit_mutable(&con->node.events.destroy, &con->node);
@@ -524,11 +526,15 @@ void container_destroy(struct sway_container *con) {
 	if (con->pending.fullscreen_mode == FULLSCREEN_GLOBAL) {
 		container_fullscreen_disable(con);
 	}
-
-	if (con->pending.parent || con->pending.workspace) {
-		container_detach(con);
+	if (root->fullscreen_global == con) {
+		root->fullscreen_global = NULL;
 	}
 
+	container_detach(con);
+
+	// Also remove from current children lists as we are freeing the container
+	container_remove_from_siblings(con, false);
+
 	if (con->view && con->view->container == con) {
 		wl_list_remove(&con->output_enter.link);
 		wl_list_remove(&con->output_leave.link);
@@ -1490,25 +1496,40 @@ void container_add_child(struct sway_container *parent,
 	node_set_dirty(&parent->node);
 }
 
-void container_detach(struct sway_container *child) {
-	if (child->pending.fullscreen_mode == FULLSCREEN_WORKSPACE) {
-		child->pending.workspace->fullscreen = NULL;
-	}
-	if (child->pending.fullscreen_mode == FULLSCREEN_GLOBAL) {
-		root->fullscreen_global = NULL;
-	}
+static void container_remove_from_siblings(struct sway_container *child, bool pending) {
+	struct sway_container_state *state = pending ? &child->pending : &child->current;
 
-	struct sway_container *old_parent = child->pending.parent;
-	struct sway_workspace *old_workspace = child->pending.workspace;
-	list_t *siblings = container_get_siblings(child);
-	if (siblings) {
-		int index = list_find(siblings, child);
+	// Remove from all possible children lists
+	list_t *siblings[] = {
+		state->parent ? state->parent->pending.children : NULL,
+		state->workspace ? state->workspace->tiling : NULL,
+		state->workspace ? state->workspace->floating : NULL,
+	};
+	for (size_t idx = 0; idx < sizeof(siblings) / sizeof(*siblings); idx++) {
+		if (!siblings[idx]) {
+			continue;
+		}
+		int index = list_find(siblings[idx], child);
 		if (index != -1) {
-			list_del(siblings, index);
+			list_del(siblings[idx], index);
 		}
 	}
 	child->pending.parent = NULL;
 	child->pending.workspace = NULL;
+}
+
+void container_detach(struct sway_container *child) {
+	struct sway_container *old_parent = child->pending.parent;
+	struct sway_workspace *old_workspace = child->pending.workspace;
+
+	if (root->fullscreen_global == child) {
+		root->fullscreen_global = NULL;
+	}
+	if (old_workspace && old_workspace->fullscreen == child) {
+		old_workspace->fullscreen = NULL;
+	}
+
+	container_remove_from_siblings(child, true);
 	container_for_each_child(child, set_workspace, NULL);
 
 	if (old_parent) {