mirror of
https://github.com/swaywm/sway
synced 2024-11-18 00:24:17 +01:00
Fix crash when unmapping a view with reapable parents
container_destroy was calling container_reap_empty, which calls container_destroy and so on. Eventually the original container_destroy would return a NULL pointer to the caller which caused a crash. This also fixes an arrange on the wrong container when moving views in and out of stacks.
This commit is contained in:
parent
33e03cb277
commit
b864ac0149
@ -177,13 +177,19 @@ static struct cmd_results *cmd_move_workspace(struct sway_container *current,
|
||||
|
||||
static void move_in_direction(struct sway_container *container,
|
||||
enum wlr_direction direction, int move_amt) {
|
||||
struct sway_container *old_parent = container->parent;
|
||||
// For simplicity, we'll arrange the entire workspace. The reason for this
|
||||
// is moving the container might reap the old parent, and container_move
|
||||
// does not return a surviving parent.
|
||||
// TODO: Make container_move return the surviving parent so we can arrange
|
||||
// just that.
|
||||
struct sway_container *old_ws = container_parent(container, C_WORKSPACE);
|
||||
container_move(container, direction, move_amt);
|
||||
struct sway_container *new_ws = container_parent(container, C_WORKSPACE);
|
||||
|
||||
struct sway_transaction *txn = transaction_create();
|
||||
arrange_windows(old_parent, txn);
|
||||
if (container->parent != old_parent) {
|
||||
arrange_windows(container->parent, txn);
|
||||
arrange_windows(old_ws, txn);
|
||||
if (new_ws != old_ws) {
|
||||
arrange_windows(new_ws, txn);
|
||||
}
|
||||
transaction_commit(txn);
|
||||
}
|
||||
|
@ -293,6 +293,47 @@ static struct sway_container *container_output_destroy(
|
||||
return &root_container;
|
||||
}
|
||||
|
||||
/**
|
||||
* Implement the actual destroy logic, without reaping.
|
||||
*/
|
||||
struct sway_container *container_destroy_noreaping(struct sway_container *con) {
|
||||
if (con == NULL) {
|
||||
return NULL;
|
||||
}
|
||||
if (con->destroying) {
|
||||
return NULL;
|
||||
}
|
||||
|
||||
// The below functions move their children to somewhere else.
|
||||
if (con->type == C_OUTPUT) {
|
||||
container_output_destroy(con);
|
||||
} else if (con->type == C_WORKSPACE) {
|
||||
// Workspaces will refuse to be destroyed if they're the last workspace
|
||||
// on their output.
|
||||
if (!container_workspace_destroy(con)) {
|
||||
wlr_log(L_ERROR, "workspace doesn't want to destroy");
|
||||
return NULL;
|
||||
}
|
||||
}
|
||||
|
||||
// At this point the container being destroyed shouldn't have any children
|
||||
// unless sway is terminating.
|
||||
if (!server.terminating) {
|
||||
if (!sway_assert(!con->children || con->children->length == 0,
|
||||
"Didn't expect to see children here")) {
|
||||
return NULL;
|
||||
}
|
||||
}
|
||||
|
||||
wl_signal_emit(&con->events.destroy, con);
|
||||
ipc_event_window(con, "close");
|
||||
|
||||
con->destroying = true;
|
||||
list_add(server.destroying_containers, con);
|
||||
|
||||
return container_remove_child(con);
|
||||
}
|
||||
|
||||
bool container_reap_empty(struct sway_container *con) {
|
||||
if (con->layout == L_FLOATING) {
|
||||
// Don't reap the magical floating container that each workspace has
|
||||
@ -306,13 +347,13 @@ bool container_reap_empty(struct sway_container *con) {
|
||||
case C_WORKSPACE:
|
||||
if (!workspace_is_visible(con) && workspace_is_empty(con)) {
|
||||
wlr_log(L_DEBUG, "Destroying workspace via reaper");
|
||||
container_destroy(con);
|
||||
container_destroy_noreaping(con);
|
||||
return true;
|
||||
}
|
||||
break;
|
||||
case C_CONTAINER:
|
||||
if (con->children->length == 0) {
|
||||
container_destroy(con);
|
||||
container_destroy_noreaping(con);
|
||||
return true;
|
||||
}
|
||||
case C_VIEW:
|
||||
@ -354,42 +395,15 @@ struct sway_container *container_flatten(struct sway_container *container) {
|
||||
* events, detach it from the tree and mark it as destroying. The container will
|
||||
* remain in memory until it's no longer used by a transaction, then it will be
|
||||
* freed via container_free().
|
||||
*
|
||||
* This function just wraps container_destroy_noreaping(), then does reaping.
|
||||
*/
|
||||
struct sway_container *container_destroy(struct sway_container *con) {
|
||||
if (con == NULL) {
|
||||
struct sway_container *parent = container_destroy_noreaping(con);
|
||||
|
||||
if (!parent) {
|
||||
return NULL;
|
||||
}
|
||||
if (con->destroying) {
|
||||
return NULL;
|
||||
}
|
||||
|
||||
// The below functions move their children to somewhere else.
|
||||
if (con->type == C_OUTPUT) {
|
||||
container_output_destroy(con);
|
||||
} else if (con->type == C_WORKSPACE) {
|
||||
// Workspaces will refuse to be destroyed if they're the last workspace
|
||||
// on their output.
|
||||
if (!container_workspace_destroy(con)) {
|
||||
return NULL;
|
||||
}
|
||||
}
|
||||
|
||||
// At this point the container being destroyed shouldn't have any children
|
||||
// unless sway is terminating.
|
||||
if (!server.terminating) {
|
||||
if (!sway_assert(!con->children || con->children->length == 0,
|
||||
"Didn't expect to see children here")) {
|
||||
return NULL;
|
||||
}
|
||||
}
|
||||
|
||||
wl_signal_emit(&con->events.destroy, con);
|
||||
ipc_event_window(con, "close");
|
||||
|
||||
struct sway_container *parent = container_remove_child(con);
|
||||
|
||||
con->destroying = true;
|
||||
list_add(server.destroying_containers, con);
|
||||
|
||||
return container_reap_empty_recursive(parent);
|
||||
}
|
||||
|
Loading…
Reference in New Issue
Block a user