diff options
author | Thomas Hebb <tommyhebb@gmail.com> | 2022-01-06 03:44:55 -0800 |
---|---|---|
committer | Simon Ser <contact@emersion.fr> | 2022-01-07 14:08:24 +0100 |
commit | 921b0a863382b70234aeb4bd589c10328e9ff042 (patch) | |
tree | fc7b25e641848c9d76effad07c61aa6aeb20bde5 | |
parent | [IPC] Add repeat delay/rate info to keyboard (diff) | |
download | sway-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.c | 15 |
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) { | |||
51 | static void seat_node_destroy(struct sway_seat_node *seat_node) { | 51 | static 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; |