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.

include/sway/desktop/transaction.h

@@ -23,6 +23,7 @@
 
 struct sway_transaction_instruction;
 struct sway_view;
+struct sway_node;
 
 /**
  * Find all dirty containers, create and commit a transaction containing them,
@@ -30,6 +31,11 @@ struct sway_view;
  */
 void transaction_commit_dirty(void);
 
+/**
+ * Remove a node that will be destroyed from transactions and dirty node lists.
+ */
+void transaction_remove_node(struct sway_node *node);
+
 /*
  * Same as transaction_commit_dirty, but signalling that this is a
  * client-initiated change has already taken effect.

include/sway/output.h

@@ -84,8 +84,6 @@ struct sway_output *output_create(struct wlr_output *wlr_output);
 
 void output_destroy(struct sway_output *output);
 
-void output_begin_destroy(struct sway_output *output);
-
 struct sway_output *output_from_wlr_output(struct wlr_output *output);
 
 struct sway_output *output_get_in_direction(struct sway_output *reference,

include/sway/tree/container.h

@@ -155,8 +155,6 @@ struct sway_container *container_create(struct sway_view *view);
 
 void container_destroy(struct sway_container *con);
 
-void container_begin_destroy(struct sway_container *con);
-
 /**
  * Search a container's descendants a container based on test criteria. Returns
  * the first container that passes the test.

include/sway/tree/node.h

@@ -39,7 +39,6 @@ struct sway_node {
 
 	struct sway_transaction_instruction *instruction;
 	size_t ntxnrefs;
-	bool destroying;
 
 	// If true, indicates that the container has pending state that differs from
 	// the current.

include/sway/tree/view.h

@@ -101,8 +101,6 @@ struct sway_view {
 	struct wl_listener foreign_close_request;
 	struct wl_listener foreign_destroy;
 
-	bool destroying;
-
 	list_t *executed_criteria; // struct criteria *
 
 	union {
@@ -296,8 +294,6 @@ bool view_init(struct sway_view *view, enum sway_view_type type,
 
 void view_destroy(struct sway_view *view);
 
-void view_begin_destroy(struct sway_view *view);
-
 /**
  * Map a view, ie. make it visible in the tree.
  *

include/sway/tree/workspace.h

@@ -62,9 +62,7 @@ struct sway_workspace *workspace_create(struct sway_output *output,
 
 void workspace_destroy(struct sway_workspace *workspace);
 
-void workspace_begin_destroy(struct sway_workspace *workspace);
-
-void workspace_consider_destroy(struct sway_workspace *ws);
+bool workspace_consider_destroy(struct sway_workspace *ws);
 
 char *workspace_next_name(const char *output_name);
 

sway/commands/move.c

@@ -611,14 +611,16 @@ static struct cmd_results *cmd_move_container(bool no_auto_back_and_forth,
 	if (old_parent) {
 		container_reap_empty(old_parent);
 	} else if (old_ws) {
-		workspace_consider_destroy(old_ws);
+		if (workspace_consider_destroy(old_ws)) {
+			old_ws = NULL;
+		}
 	}
 
 	// arrange windows
 	if (root->fullscreen_global) {
 		arrange_root();
 	} else {
-		if (old_ws && !old_ws->node.destroying) {
+		if (old_ws) {
 			arrange_workspace(old_ws);
 		}
 		arrange_node(node_get_parent(destination));
@@ -753,7 +755,9 @@ static struct cmd_results *cmd_move_in_direction(
 	if (old_parent) {
 		container_reap_empty(old_parent);
 	} else if (old_ws) {
-		workspace_consider_destroy(old_ws);
+		if (workspace_consider_destroy(old_ws)) {
+			old_ws = NULL;
+		}
 	}
 
 	struct sway_workspace *new_ws = container->pending.workspace;
@@ -761,7 +765,9 @@ static struct cmd_results *cmd_move_in_direction(
 	if (root->fullscreen_global) {
 		arrange_root();
 	} else {
-		arrange_workspace(old_ws);
+		if (old_ws) {
+			arrange_workspace(old_ws);
+		}
 		if (new_ws != old_ws) {
 			arrange_workspace(new_ws);
 		}

sway/desktop/launcher.c

@@ -153,7 +153,7 @@ static void ctx_handle_node_destroy(struct wl_listener *listener, void *data) {
 		// same output
 		free(ctx->fallback_name);
 		ctx->fallback_name = strdup(ws->name);
-		if (!ws->output || ws->output->node.destroying) {
+		if (!ws->output) {
 			// If the output is being destroyed it would be pointless to track
 			// If the output is being disabled, we'll find out if it's still
 			// disabled when we try to match it.

sway/desktop/output.c

@@ -428,7 +428,7 @@ void force_modeset(void) {
 	apply_stored_output_configs();
 }
 
-static void begin_destroy(struct sway_output *output) {
+static void output_teardown(struct sway_output *output) {
 
 	wl_list_remove(&output->layout_destroy.link);
 	wl_list_remove(&output->destroy.link);
@@ -444,7 +444,6 @@ static void begin_destroy(struct sway_output *output) {
 	if (output->enabled) {
 		output_disable(output);
 	}
-	output_begin_destroy(output);
 	wl_list_remove(&output->link);
 
 	output->wlr_output->data = NULL;
@@ -453,17 +452,19 @@ static void begin_destroy(struct sway_output *output) {
 	wl_event_source_remove(output->repaint_timer);
 	output->repaint_timer = NULL;
 
+	output_destroy(output);
+
 	request_modeset();
 }
 
 static void handle_destroy(struct wl_listener *listener, void *data) {
 	struct sway_output *output = wl_container_of(listener, output, destroy);
-	begin_destroy(output);
+	output_teardown(output);
 }
 
 static void handle_layout_destroy(struct wl_listener *listener, void *data) {
 	struct sway_output *output = wl_container_of(listener, output, layout_destroy);
-	begin_destroy(output);
+	output_teardown(output);
 }
 
 static void handle_present(struct wl_listener *listener, void *data) {

sway/desktop/transaction.c

@@ -59,22 +59,6 @@ static void transaction_destroy(struct sway_transaction *transaction) {
 		if (node->instruction == instruction) {
 			node->instruction = NULL;
 		}
-		if (node->destroying && node->ntxnrefs == 0) {
-			switch (node->type) {
-			case N_ROOT:
-				sway_assert(false, "Never reached");
-				break;
-			case N_OUTPUT:
-				output_destroy(node->sway_output);
-				break;
-			case N_WORKSPACE:
-				workspace_destroy(node->sway_workspace);
-				break;
-			case N_CONTAINER:
-				container_destroy(node->sway_container);
-				break;
-			}
-		}
 		free(instruction);
 	}
 	list_free(transaction->instructions);
@@ -239,7 +223,7 @@ static void apply_container_state(struct sway_container *container,
 
 	if (view) {
 		if (view->saved_surface_tree) {
-			if (!container->node.destroying || container->node.ntxnrefs == 1) {
+			if (container->node.ntxnrefs == 1) {
 				view_remove_saved_buffer(view);
 			}
 		}
@@ -788,9 +772,6 @@ static bool should_configure(struct sway_node *node,
 	if (!node_is_view(node)) {
 		return false;
 	}
-	if (node->destroying) {
-		return false;
-	}
 	if (!instruction->server_request) {
 		return false;
 	}
@@ -825,7 +806,7 @@ static void transaction_commit(struct sway_transaction *transaction) {
 		struct sway_transaction_instruction *instruction =
 			transaction->instructions->items[i];
 		struct sway_node *node = instruction->node;
-		bool hidden = node_is_view(node) && !node->destroying &&
+		bool hidden = node_is_view(node) &&
 			!view_is_visible(node->sway_container->view);
 		if (should_configure(node, instruction)) {
 			instruction->serial = view_configure(node->sway_container->view,
@@ -967,3 +948,39 @@ void transaction_commit_dirty(void) {
 void transaction_commit_dirty_client(void) {
 	_transaction_commit_dirty(false);
 }
+
+static void _transaction_remove_node(struct sway_transaction *transaction,
+		struct sway_node *node) {
+	if (!transaction || !node) {
+		return;
+	}
+	for (int idx = 0; idx < transaction->instructions->length; idx++) {
+		struct sway_transaction_instruction *instruction =
+			transaction->instructions->items[idx];
+		struct sway_node *n = instruction->node;
+		if (n != node) {
+			continue;
+		}
+
+		n->ntxnrefs--;
+		n->instruction = NULL;
+		free(instruction);
+		list_del(transaction->instructions, idx);
+		idx--;
+	}
+}
+
+void transaction_remove_node(struct sway_node *node) {
+	_transaction_remove_node(server.pending_transaction, node);
+	_transaction_remove_node(server.queued_transaction, node);
+
+	for (int idx = 0; idx < server.dirty_nodes->length; idx++) {
+		struct sway_node *n = server.dirty_nodes->items[idx];
+		if (n != node) {
+			continue;
+		}
+		n->dirty = false;
+		list_del(server.dirty_nodes, idx);
+		idx--;
+	}
+}

sway/desktop/xdg_shell.c

@@ -535,7 +535,7 @@ static void handle_destroy(struct wl_listener *listener, void *data) {
 	if (view->xdg_decoration) {
 		view->xdg_decoration->view = NULL;
 	}
-	view_begin_destroy(view);
+	view_destroy(view);
 }
 
 struct sway_view *view_from_wlr_xdg_surface(

sway/desktop/xwayland.c

@@ -482,7 +482,7 @@ static void handle_destroy(struct wl_listener *listener, void *data) {
 	wl_list_remove(&xwayland_view->associate.link);
 	wl_list_remove(&xwayland_view->dissociate.link);
 	wl_list_remove(&xwayland_view->override_redirect.link);
-	view_begin_destroy(&xwayland_view->view);
+	view_destroy(&xwayland_view->view);
 }
 
 static void handle_unmap(struct wl_listener *listener, void *data) {

sway/main.c

@@ -108,20 +108,6 @@ static void log_kernel(void) {
 	pclose(f);
 }
 
-static bool detect_suid(void) {
-	if (geteuid() != 0 && getegid() != 0) {
-		return false;
-	}
-
-	if (getuid() == geteuid() && getgid() == getegid()) {
-		return false;
-	}
-
-	sway_log(SWAY_ERROR, "SUID operation is no longer supported, refusing to start. "
-			"This check will be removed in a future release.");
-	return true;
-}
-
 static void restore_nofile_limit(void) {
 	if (original_nofile_rlimit.rlim_cur == 0) {
 		return;
@@ -292,11 +278,6 @@ int main(int argc, char **argv) {
 		}
 	}
 
-	// SUID operation is deprecated, so block it for now.
-	if (detect_suid()) {
-		exit(EXIT_FAILURE);
-	}
-
 	// Since wayland requires XDG_RUNTIME_DIR to be set, abort with just the
 	// clear error message (when not running as an IPC client).
 	if (!getenv("XDG_RUNTIME_DIR") && optind == argc) {

sway/tree/container.c

@@ -55,7 +55,7 @@ static void handle_destroy(
 	struct sway_container *con = wl_container_of(
 			listener, con, output_handler_destroy);
 
-	container_begin_destroy(con);
+	container_destroy(con);
 }
 
 static bool handle_point_accepts_input(
@@ -500,11 +500,48 @@ 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 (!sway_assert(con->node.destroying,
-				"Tried to free container which wasn't marked as destroying")) {
-		return;
+	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.workspace && con->pending.workspace->fullscreen == con) {
+		con->pending.workspace->fullscreen = NULL;
+	}
+	if (con->current.workspace && con->current.workspace->fullscreen == con) {
+		con->current.workspace->fullscreen = NULL;
+	}
+
+	wl_signal_emit_mutable(&con->node.events.destroy, &con->node);
+
+	container_end_mouse_operation(con);
+
+	if (con->scratchpad) {
+		root_scratchpad_remove_container(con);
+	}
+
+	if (con->pending.fullscreen_mode == FULLSCREEN_GLOBAL) {
+		container_fullscreen_disable(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);
+		wl_list_remove(&con->output_handler_destroy.link);
+	}
+
+	transaction_remove_node(&con->node);
 	if (!sway_assert(con->node.ntxnrefs == 0, "Tried to free container "
 				"which is still referenced by transactions")) {
 		return;
@@ -520,9 +557,6 @@ void container_destroy(struct sway_container *con) {
 	if (con->view && con->view->container == con) {
 		con->view->container = NULL;
 		wlr_scene_node_destroy(&con->output_handler->node);
-		if (con->view->destroying) {
-			view_destroy(con->view);
-		}
 	}
 
 	scene_node_disown_children(con->content_tree);
@@ -530,45 +564,6 @@ void container_destroy(struct sway_container *con) {
 	free(con);
 }
 
-void container_begin_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) {
-		con->pending.workspace->fullscreen = NULL;
-	}
-	if (con->scratchpad && con->pending.fullscreen_mode == FULLSCREEN_GLOBAL) {
-		container_fullscreen_disable(con);
-	}
-
-	wl_signal_emit_mutable(&con->node.events.destroy, &con->node);
-
-	container_end_mouse_operation(con);
-
-	con->node.destroying = true;
-	node_set_dirty(&con->node);
-
-	if (con->scratchpad) {
-		root_scratchpad_remove_container(con);
-	}
-
-	if (con->pending.fullscreen_mode == FULLSCREEN_GLOBAL) {
-		container_fullscreen_disable(con);
-	}
-
-	if (con->pending.parent || con->pending.workspace) {
-		container_detach(con);
-	}
-
-	if (con->view && con->view->container == con) {
-		wl_list_remove(&con->output_enter.link);
-		wl_list_remove(&con->output_leave.link);
-		wl_list_remove(&con->output_handler_destroy.link);
-	}
-}
-
 void container_reap_empty(struct sway_container *con) {
 	if (con->view) {
 		return;
@@ -579,7 +574,7 @@ void container_reap_empty(struct sway_container *con) {
 			return;
 		}
 		struct sway_container *parent = con->pending.parent;
-		container_begin_destroy(con);
+		container_destroy(con);
 		con = parent;
 	}
 	if (ws) {
@@ -595,7 +590,7 @@ struct sway_container *container_flatten(struct sway_container *container) {
 		struct sway_container *child = container->pending.children->items[0];
 		struct sway_container *parent = container->pending.parent;
 		container_replace(container, child);
-		container_begin_destroy(container);
+		container_destroy(container);
 		container = parent;
 	}
 	return container;
@@ -1501,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) {

sway/tree/output.c

@@ -2,6 +2,7 @@
 #include <ctype.h>
 #include <string.h>
 #include <strings.h>
+#include "sway/desktop/transaction.h"
 #include "sway/ipc-server.h"
 #include "sway/layers.h"
 #include "sway/output.h"
@@ -235,7 +236,7 @@ static void output_evacuate(struct sway_output *output) {
 			}
 
 			if (workspace_num_sticky_containers(workspace) == 0) {
-				workspace_begin_destroy(workspace);
+				workspace_destroy(workspace);
 				continue;
 			}
 		}
@@ -253,27 +254,6 @@ static void output_evacuate(struct sway_output *output) {
 	}
 }
 
-void output_destroy(struct sway_output *output) {
-	if (!sway_assert(output->node.destroying,
-				"Tried to free output which wasn't marked as destroying")) {
-		return;
-	}
-	if (!sway_assert(output->wlr_output == NULL,
-				"Tried to free output which still had a wlr_output")) {
-		return;
-	}
-	if (!sway_assert(output->node.ntxnrefs == 0, "Tried to free output "
-				"which is still referenced by transactions")) {
-		return;
-	}
-
-	destroy_scene_layers(output);
-	list_free(output->workspaces);
-	list_free(output->current.workspaces);
-	wlr_color_transform_unref(output->color_transform);
-	free(output);
-}
-
 void output_disable(struct sway_output *output) {
 	if (!sway_assert(output->enabled, "Expected an enabled output")) {
 		return;
@@ -295,15 +275,24 @@ void output_disable(struct sway_output *output) {
 	output_evacuate(output);
 }
 
-void output_begin_destroy(struct sway_output *output) {
+void output_destroy(struct sway_output *output) {
 	if (!sway_assert(!output->enabled, "Expected a disabled output")) {
 		return;
 	}
-	sway_log(SWAY_DEBUG, "Destroying output '%s'", output->wlr_output->name);
 	wl_signal_emit_mutable(&output->node.events.destroy, &output->node);
 
-	output->node.destroying = true;
-	node_set_dirty(&output->node);
+	transaction_remove_node(&output->node);
+
+	if (!sway_assert(output->node.ntxnrefs == 0, "Tried to free output "
+				"which is still referenced by transactions")) {
+		return;
+	}
+
+	destroy_scene_layers(output);
+	list_free(output->workspaces);
+	list_free(output->current.workspaces);
+	wlr_color_transform_unref(output->color_transform);
+	free(output);
 }
 
 struct sway_output *output_from_wlr_output(struct wlr_output *output) {

sway/tree/view.c

@@ -70,11 +70,10 @@ bool view_init(struct sway_view *view, enum sway_view_type type,
 }
 
 void view_destroy(struct sway_view *view) {
-	if (!sway_assert(view->surface == NULL, "Tried to free mapped view")) {
+	if (!sway_assert(view->surface == NULL, "Tried to destroy a mapped view")) {
 		return;
 	}
-	if (!sway_assert(view->destroying,
-				"Tried to free view which wasn't marked as destroying")) {
+	if (!sway_assert(view->surface == NULL, "Tried to free mapped view")) {
 		return;
 	}
 	if (!sway_assert(view->container == NULL,
@@ -95,17 +94,6 @@ void view_destroy(struct sway_view *view) {
 	}
 }
 
-void view_begin_destroy(struct sway_view *view) {
-	if (!sway_assert(view->surface == NULL, "Tried to destroy a mapped view")) {
-		return;
-	}
-	view->destroying = true;
-
-	if (!view->container) {
-		view_destroy(view);
-	}
-}
-
 const char *view_get_title(struct sway_view *view) {
 	if (view->impl->get_string_prop) {
 		return view->impl->get_string_prop(view, VIEW_PROP_TITLE);
@@ -935,17 +923,19 @@ void view_unmap(struct sway_view *view) {
 
 	struct sway_container *parent = view->container->pending.parent;
 	struct sway_workspace *ws = view->container->pending.workspace;
-	container_begin_destroy(view->container);
+	container_destroy(view->container);
 	if (parent) {
 		container_reap_empty(parent);
 	} else if (ws) {
-		workspace_consider_destroy(ws);
+		if (workspace_consider_destroy(ws)) {
+			ws = NULL;
+		}
 	}
 
 	if (root->fullscreen_global) {
 		// Container may have been a child of the root fullscreen container
 		arrange_root();
-	} else if (ws && !ws->node.destroying) {
+	} else if (ws) {
 		arrange_workspace(ws);
 		workspace_detect_urgent(ws);
 	}
@@ -1099,9 +1089,6 @@ void view_update_title(struct sway_view *view, bool force) {
 }
 
 bool view_is_visible(struct sway_view *view) {
-	if (view->container->node.destroying) {
-		return false;
-	}
 	struct sway_workspace *workspace = view->container->pending.workspace;
 	if (!workspace && view->container->pending.fullscreen_mode != FULLSCREEN_GLOBAL) {
 		bool fs_global_descendant = false;

sway/tree/workspace.c

@@ -5,6 +5,7 @@
 #include <stdio.h>
 #include <strings.h>
 #include "stringop.h"
+#include "sway/desktop/transaction.h"
 #include "sway/input/input-manager.h"
 #include "sway/input/cursor.h"
 #include "sway/input/seat.h"
@@ -134,10 +135,15 @@ struct sway_workspace *workspace_create(struct sway_output *output,
 }
 
 void workspace_destroy(struct sway_workspace *workspace) {
-	if (!sway_assert(workspace->node.destroying,
-				"Tried to free workspace which wasn't marked as destroying")) {
-		return;
+	sway_log(SWAY_DEBUG, "Destroying workspace '%s'", workspace->name);
+	ipc_event_workspace(NULL, workspace, "empty"); // intentional
+	wl_signal_emit_mutable(&workspace->node.events.destroy, &workspace->node);
+
+	if (workspace->output) {
+		workspace_detach(workspace);
 	}
+	transaction_remove_node(&workspace->node);
+
 	if (!sway_assert(workspace->node.ntxnrefs == 0, "Tried to free workspace "
 				"which is still referenced by transactions")) {
 		return;
@@ -158,36 +164,25 @@ void workspace_destroy(struct sway_workspace *workspace) {
 	free(workspace);
 }
 
-void workspace_begin_destroy(struct sway_workspace *workspace) {
-	sway_log(SWAY_DEBUG, "Destroying workspace '%s'", workspace->name);
-	ipc_event_workspace(NULL, workspace, "empty"); // intentional
-	wl_signal_emit_mutable(&workspace->node.events.destroy, &workspace->node);
-
-	if (workspace->output) {
-		workspace_detach(workspace);
-	}
-	workspace->node.destroying = true;
-	node_set_dirty(&workspace->node);
-}
-
-void workspace_consider_destroy(struct sway_workspace *ws) {
+bool workspace_consider_destroy(struct sway_workspace *ws) {
 	if (ws->tiling->length || ws->floating->length) {
-		return;
+		return false;
 	}
 
 	if (ws->output && output_get_active_workspace(ws->output) == ws) {
-		return;
+		return false;
 	}
 
 	struct sway_seat *seat;
 	wl_list_for_each(seat, &server.input->seats, link) {
 		struct sway_node *node = seat_get_focus_inactive(seat, &root->node);
 		if (node == &ws->node) {
-			return;
+			return false;
 		}
 	}
 
-	workspace_begin_destroy(ws);
+	workspace_destroy(ws);
+	return true;
 }
 
 static bool workspace_valid_on_output(const char *output_name,
@@ -596,9 +591,6 @@ bool workspace_switch(struct sway_workspace *workspace) {
 }
 
 bool workspace_is_visible(struct sway_workspace *ws) {
-	if (ws->node.destroying) {
-		return false;
-	}
 	return output_get_active_workspace(ws->output) == ws;
 }