Skip to content

atspi-common: register_tree panics (then aborts) when adapters become active out of order: push_adapter doesn't keep the list sorted for adapter_index's binary search #820

Closed

Description

@datstma

What happens

AppContext in accesskit_atspi_common finds adapters with a binary search, but push_adapter appends to the end, so the list is only sorted if adapters are added in order of id:

// adapters/atspi-common/src/context.rs
pub(crate) fn adapter_index(&self, id: usize) -> Result<usize, usize> {
    self.adapters.binary_search_by(|adapter| adapter.0.cmp(&id))
}

pub(crate) fn push_adapter(&mut self, id: usize, context: &Arc<Context>) {
    self.adapters.push((id, Arc::clone(context)));
}

With accesskit_unix, an adapter is pushed when it goes from Pending to Active, which happens in the application's next update_if_active for that window after AT-SPI is enabled. With several windows, that is whatever order the application updates them in, not the order they were created, so the list can end up out of order (for example ids [1, 2, 0]). A binary search then misses an adapter that is in the list, and register_tree panics on

let adapter_index = app_context.adapter_index(self.id).unwrap();

while holding the app context's write lock. Unwinding drops the adapter, whose Drop calls write_app_context() on the now poisoned lock and panics again, so the process aborts:

panicked at accesskit_atspi_common-0.18.1/src/adapter.rs:461
panicked at accesskit_atspi_common-0.18.1/src/context.rs:85
panic in a destructor during cleanup
thread caused non-unwinding panic. aborting.

remove_adapter has the same problem in a quieter form: on an unsorted list it can fail to find the adapter and leave it in the list.

What should happen

Adapters can become active in any order without the lookup failing.

How to reproduce

We hit it in an application with several windows, each with an accesskit_winit adapter, on Ubuntu 26.04 (GNOME 50, Wayland and XWayland) with AT-SPI enabled, when the windows were updated in a different order than they were created. It depends on that order, so we don't have a small program that hits it every time; the lookup itself shows it. With the adapters pushed in the order 1, 2, 0:

let adapters: Vec<(usize, ())> = vec![(1, ()), (2, ()), (0, ())];
let found = adapters.binary_search_by(|adapter| adapter.0.cmp(&0));
println!("{found:?}"); // Err(0): adapter 0 is in the list but isn't found

Suggested fix

Keep the list sorted when adding:

pub(crate) fn push_adapter(&mut self, id: usize, context: &Arc<Context>) {
    match self.adapter_index(id) {
        Ok(index) => self.adapters[index] = (id, Arc::clone(context)),
        Err(index) => self.adapters.insert(index, (id, Arc::clone(context))),
    }
}

We're running with exactly this change (on 0.18.1, which egui 0.36 uses) and haven't seen the crash since. The code is the same on main (context.rs lines 109–115, adapter.rs line 463).

Versions: accesskit_atspi_common 0.18.1 through accesskit_unix 0.21.1 and accesskit_winit; also present in 0.21.0 and on main.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions