aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorLibravatar Thomas Hebb <tommyhebb@gmail.com>2022-01-06 03:44:55 -0800
committerLibravatar Simon Ser <contact@emersion.fr>2022-01-07 14:08:24 +0100
commit921b0a863382b70234aeb4bd589c10328e9ff042 (patch)
treefc7b25e641848c9d76effad07c61aa6aeb20bde5
parent[IPC] Add repeat delay/rate info to keyboard (diff)
downloadsway-921b0a863382b70234aeb4bd589c10328e9ff042.tar.gz
sway-921b0a863382b70234aeb4bd589c10328e9ff042.tar.zst
sway-921b0a863382b70234aeb4bd589c10328e9ff042.zip
input/seat: unset has_focus when focus_stack becomes empty
We currently track the focus of a seat in two ways: we use a list called focus_stack to track the order in which nodes have been focused, with the first node representing what's currently focused, and we use a variable called has_focus to indicate whether anything has focus--i.e. whether we should actually treat that first node as focused at any given time. In a number of places, we treat has_focus as implying that a focused node exists. If it's true, we attempt to dereference the return value of seat_get_focus(), our helper function for getting the first node in focus_list, with no further checks. But this isn't quite correct with the current implementation of seat_get_focus(): not only does it return NULL when has_focus is false, it also returns NULL when focus_stack contains no items. In most cases, focus_stack never becomes empty and so this doesn't matter at all. Since focus_stack stores a history of focused nodes, we rarely remove nodes from it. The exception to this is when a node itself goes away. In that case, we call seat_node_destroy() to remove it from focus_stack and free it. But we don't unset has_focus if we've removed the final node! This lets us get into a state where has_focus is true but seat_get_focus() returns NULL, leading to a segfault when we try to dereference it. Fix the issue both by updating has_focus in seat_node_destroy() and by adding an assertion in seat_get_focus() that ensures focus_stack and has_focus are in sync, which will make it easier to track down similar issues in the future. Fixes #6395. [1] There's some discussion in #1585 from when this was implemented about whether has_focus is actually necessary; it's possible we could remove it entirely, but for the moment this is the architecture we have.
-rw-r--r--sway/input/seat.c15
1 files changed, 12 insertions, 3 deletions
diff --git a/sway/input/seat.c b/sway/input/seat.c
index c5c8459e..ce933b66 100644
--- a/sway/input/seat.c
+++ b/sway/input/seat.c
@@ -51,6 +51,16 @@ static void seat_device_destroy(struct sway_seat_device *seat_device) {
51static void seat_node_destroy(struct sway_seat_node *seat_node) { 51static void seat_node_destroy(struct sway_seat_node *seat_node) {
52 wl_list_remove(&seat_node->destroy.link); 52 wl_list_remove(&seat_node->destroy.link);
53 wl_list_remove(&seat_node->link); 53 wl_list_remove(&seat_node->link);
54
55 /*
56 * This is the only time we remove items from the focus stack without
57 * immediately re-adding them. If we just removed the last thing,
58 * mark that nothing has focus anymore.
59 */
60 if (wl_list_empty(&seat_node->seat->focus_stack)) {
61 seat_node->seat->has_focus = false;
62 }
63
54 free(seat_node); 64 free(seat_node);
55} 65}
56 66
@@ -1415,9 +1425,8 @@ struct sway_node *seat_get_focus(struct sway_seat *seat) {
1415 if (!seat->has_focus) { 1425 if (!seat->has_focus) {
1416 return NULL; 1426 return NULL;
1417 } 1427 }
1418 if (wl_list_empty(&seat->focus_stack)) { 1428 sway_assert(!wl_list_empty(&seat->focus_stack),
1419 return NULL; 1429 "focus_stack is empty, but has_focus is true");
1420 }
1421 struct sway_seat_node *current = 1430 struct sway_seat_node *current =
1422 wl_container_of(seat->focus_stack.next, current, link); 1431 wl_container_of(seat->focus_stack.next, current, link);
1423 return current->node; 1432 return current->node;