mirrors/foot
1
0
Fork 0
mirror of https://codeberg.org/dnkl/foot.git synced 2026-02-05 04:06:08 -05:00
Code Issues Projects Releases Packages Wiki Activity Actions

terminal: don’t unref a not-yet-referenced key-binding set

Browse source 
Key-binding sets are bound to a seat/configuration pair. The conf
reference is done when a new terminal instance is created.

When that same terminal instance is destroyed, the key binding set is
unref:ed.

If the terminal instance is destroyed *before* the key binding set has
been referenced, we’ll still unref it. This creates an imbalance.

In particular, when the there is exactly one other terminal instance
referencing that same key binding set, that terminal instance will
trigger a foot server crash as soon as it receives a key press/release
event. This happens because the next-to-last terminal instance brought
the reference count of the binding set down to 0, causing it to be
free:d.

Thus, we *must* reference the binding set *before* we can error
out (when instantiating a new terminal instance).

At this point, we don’t yet have a valid terminal instance. But,
that’s ok, because all the key_binding_new_for_term() did with the
terminal instance was get the "struct wayland" and "struct config"
pointers. So, rename the function and simply pass these pointers
explicitly.

Similarly, change key_binding_for() to take a "struct config" pointer,
rather than a "struct terminal" pointer.

Also rename key_binding_unref_term() -> key_binding_unref().
This commit is contained in:
Daniel Eklöf 2022-09-05 19:23:40 +02:00
parent c311229b9e
commit d2e67689ea
No known key found for this signature in database
GPG key ID: 5BBD4992C116573F
6 changed files with 35 additions and 31 deletions
Download patch file Download diff file Expand all files Collapse all files

6
CHANGELOG.md
View file

@ -60,6 +60,12 @@
### Deprecated
### Removed
### Fixed
* Crash in `foot --server` on key press, after another `footclient`
has terminated very early (for example, by trying to launch a
non-existing shell/client).
### Security
### Contributors

4
input.c
View file

@ -1406,7 +1406,7 @@ key_press_release(struct seat *seat, struct terminal *term, uint32_t serial,
seat->kbd.xkb_keymap, key, layout_idx, 0, &raw_syms);
const struct key_binding_set *bindings = key_binding_for(
seat->wayl->key_binding_manager, term, seat);
seat->wayl->key_binding_manager, term->conf, seat);
xassert(bindings != NULL);
if (pressed) {
@ -2335,7 +2335,7 @@ wl_pointer_button(void *data, struct wl_pointer *wl_pointer,
/* Seat has keyboard - use mouse bindings *with* modifiers */
const struct key_binding_set *bindings = key_binding_for(
wayl->key_binding_manager, term, seat);
wayl->key_binding_manager, term->conf, seat);
xassert(bindings != NULL);
xkb_mod_mask_t mods;

22
key-binding.c
View file

@ -80,17 +80,14 @@ key_binding_new_for_seat(struct key_binding_manager *mgr,
}
void
key_binding_new_for_term(struct key_binding_manager *mgr,
const struct terminal *term)
key_binding_new_for_conf(struct key_binding_manager *mgr,
const struct wayland *wayl, const struct config *conf)
{
const struct config *conf = term->conf;
const struct wayland *wayl = term->wl;
tll_foreach(wayl->seats, it) {
struct seat *seat = &it->item;
struct key_set *existing =
(struct key_set *)key_binding_for(mgr, term, seat);
(struct key_set *)key_binding_for(mgr, conf, seat);
if (existing != NULL) {
existing->conf_ref_count++;
@ -116,21 +113,19 @@ key_binding_new_for_term(struct key_binding_manager *mgr,
/* Chances are high this set will be requested next */
mgr->last_used_set = &tll_back(mgr->binding_sets);
LOG_DBG("new (term): set=%p, seat=%p, conf=%p, ref-count=1",
LOG_DBG("new (conf): set=%p, seat=%p, conf=%p, ref-count=1",
(void *)&tll_back(mgr->binding_sets),
(void *)set.seat, (void *)set.conf);
}
LOG_DBG("new (term): total number of sets: %zu",
LOG_DBG("new (conf): total number of sets: %zu",
tll_length(mgr->binding_sets));
}
struct key_binding_set * NOINLINE
key_binding_for(struct key_binding_manager *mgr, const struct terminal *term,
key_binding_for(struct key_binding_manager *mgr, const struct config *conf,
const struct seat *seat)
{
const struct config *conf = term->conf;
struct key_set *last_used = mgr->last_used_set;
if (last_used != NULL &&
last_used->conf == conf &&
@ -192,11 +187,8 @@ key_binding_remove_seat(struct key_binding_manager *mgr,
}
void
key_binding_unref_term(struct key_binding_manager *mgr,
const struct terminal *term)
key_binding_unref(struct key_binding_manager *mgr, const struct config *conf)
{
const struct config *conf = term->conf;
tll_foreach(mgr->binding_sets, it) {
struct key_set *set = &it->item;

14
key-binding.h
View file

@ -110,6 +110,7 @@ typedef tll(struct key_binding) key_binding_list_t;
struct terminal;
struct seat;
struct wayland;
struct key_binding_set {
key_binding_list_t key;
@ -127,20 +128,21 @@ void key_binding_manager_destroy(struct key_binding_manager *mgr);
void key_binding_new_for_seat(
struct key_binding_manager *mgr, const struct seat *seat);
void key_binding_new_for_term(
struct key_binding_manager *mgr, const struct terminal *term);
void key_binding_new_for_conf(
struct key_binding_manager *mgr, const struct wayland *wayl,
const struct config *conf);
/* Returns the set of key bindings associated with this seat/term pair */
/* Returns the set of key bindings associated with this seat/conf pair */
struct key_binding_set *key_binding_for(
struct key_binding_manager *mgr, const struct terminal *term,
struct key_binding_manager *mgr, const struct config *conf,
const struct seat *seat);
/* Remove all key bindings tied to the specified seat */
void key_binding_remove_seat(
struct key_binding_manager *mgr, const struct seat *seat);
void key_binding_unref_term(
struct key_binding_manager *mgr, const struct terminal *term);
void key_binding_unref(
struct key_binding_manager *mgr, const struct config *conf);
void key_binding_load_keymap(
struct key_binding_manager *mgr, const struct seat *seat);

9
pgo/pgo.c
View file

@ -178,15 +178,16 @@ static bool kbd_initialized = false;
struct key_binding_set *
key_binding_for(
struct key_binding_manager *mgr, const struct terminal *term,
struct key_binding_manager *mgr, const struct config *conf,
const struct seat *seat)
{
return &kbd;
}
void
key_binding_new_for_term(
struct key_binding_manager *mgr, const struct terminal *term)
key_binding_new_for_conf(
struct key_binding_manager *mgr, const struct wayland *wayl,
const struct config *conf)
{
if (!kbd_initialized) {
kbd_initialized = true;
@ -201,7 +202,7 @@ key_binding_new_for_term(
}
void
key_binding_unref_term(struct key_binding_manager *mgr, const struct terminal *term)
key_binding_unref(struct key_binding_manager *mgr, const struct config *conf)
{
}

11
terminal.c
View file

@ -1089,6 +1089,11 @@ term_init(const struct config *conf, struct fdm *fdm, struct reaper *reaper,
goto close_fds;
}
/* Need to register *very* early (before the first “goto err”), to
* ensure term_destroy() doesn’t unref a key-binding we haven’t
* yet ref:d */
key_binding_new_for_conf(wayl->key_binding_manager, wayl, conf);
int ptmx_flags;
if ((ptmx_flags = fcntl(ptmx, F_GETFL)) < 0 ||
fcntl(ptmx, F_SETFL, ptmx_flags | O_NONBLOCK) < 0)
@ -1266,8 +1271,6 @@ term_init(const struct config *conf, struct fdm *fdm, struct reaper *reaper,
memcpy(term->colors.table, term->conf->colors.table, sizeof(term->colors.table));
key_binding_new_for_term(wayl->key_binding_manager, term);
/* Initialize the Wayland window backend */
if ((term->window = wayl_win_init(term, token)) == NULL)
goto err;
@ -1583,7 +1586,7 @@ term_destroy(struct terminal *term)
if (term == NULL)
return 0;
key_binding_unref_term(term->wl->key_binding_manager, term);
key_binding_unref(term->wl->key_binding_manager, term->conf);
tll_foreach(term->wl->terms, it) {
if (it->item == term) {
@ -2885,7 +2888,7 @@ term_mouse_grabbed(const struct terminal *term, const struct seat *seat)
get_current_modifiers(seat, &mods, NULL, 0);
const struct key_binding_set *bindings =
key_binding_for(term->wl->key_binding_manager, term, seat);
key_binding_for(term->wl->key_binding_manager, term->conf, seat);
const xkb_mod_mask_t override_modmask = bindings->selection_overrides;
bool override_mods_pressed = (mods & override_modmask) == override_modmask;