rust-x-bindings / rust-xcb

Rust bindings and wrapper for XCB.
MIT License
162 stars 64 forks source link

Crash on unresolved xproto event #141

Closed pr2502 closed 2 years ago

pr2502 commented 2 years ago

This occurred when running using the program as a window manager. It seems to have received the x::StoreColors request as an event which it then attempted and failed to parse. I'm not sure whether receiving requests as events is expected when running as a window manager or whether this is a bug earlier in the parsing and the response_type is nonsensical.

The issue is reproducible for me when in version https://github.com/pr2502/mwm/tree/9498d9640f199ea6080aa11542973bfbca93763f running ./run.sh to run a debug build in Xephyr and then opening rofi with DISPLAY=:3 rofi -show window.

Backtrace

thread 'main' panicked at 'internal error: entered unreachable code: Could not resolve xproto Event with response_type 89 and first_event 0', ./target/debug/build/xcb-f88379ca782dda16/out/xproto.rs:5074:18
stack backtrace:
   0: rust_begin_unwind
             at /rustc/8f3238f898163f09726c3d2b2cc9bafb09da26f3/library/std/src/panicking.rs:498:5
   1: core::panicking::panic_fmt
             at /rustc/8f3238f898163f09726c3d2b2cc9bafb09da26f3/library/core/src/panicking.rs:107:14
   2: ::resolve_wire_event
             at ./target/debug/build/xcb-f88379ca782dda16/out/xproto.rs:5074:18
   3: xcb::event::resolve_event
             at home/.cargo/git/checkouts/rust-xcb-35c0c23e290e4816/afeaee7/src/event.rs:233:14
   4: xcb::base::Connection::handle_wait_for_event
             at home/.cargo/git/checkouts/rust-xcb-35c0c23e290e4816/afeaee7/src/base.rs:1134:16
   5: xcb::base::Connection::wait_for_event
             at home/.cargo/git/checkouts/rust-xcb-35c0c23e290e4816/afeaee7/src/base.rs:1186:13
   6: mwm_xcb::xcb_event_systems::wait_for_xcb_events
             at ./mwm_xcb/src/xcb_event_systems.rs:26:14
   7: core::ops::function::Fn::call
             at /rustc/8f3238f898163f09726c3d2b2cc9bafb09da26f3/library/core/src/ops/function.rs:70:5
   8: core::ops::function::impls:: for &F>::call_mut
             at /rustc/8f3238f898163f09726c3d2b2cc9bafb09da26f3/library/core/src/ops/function.rs:247:13
   9: >::run
             at home/.cargo/registry/src/github.com-1ecc6299db9ec823/bevy_ecs-0.5.0/src/system/into_system.rs:207:21
  10:  as bevy_ecs::system::system::System>::run_unsafe
             at home/.cargo/registry/src/github.com-1ecc6299db9ec823/bevy_ecs-0.5.0/src/system/into_system.rs:147:19
  11:  as bevy_ecs::system::system::System>::run_unsafe
             at home/.cargo/registry/src/github.com-1ecc6299db9ec823/bevy_ecs-0.5.0/src/system/system_chaining.rs:54:19
  12: bevy_ecs::schedule::executor_parallel::ParallelExecutor::prepare_systems::{{closure}}
             at home/.cargo/registry/src/github.com-1ecc6299db9ec823/bevy_ecs-0.5.0/src/schedule/executor_parallel.rs:200:30
  13:  as core::future::future::Future>::poll
             at /rustc/8f3238f898163f09726c3d2b2cc9bafb09da26f3/library/core/src/future/mod.rs:84:19
  14: async_executor::Executor::spawn::{{closure}}
             at home/.cargo/registry/src/github.com-1ecc6299db9ec823/async-executor-1.4.1/src/lib.rs:144:19
  15:  as core::future::future::Future>::poll
             at /rustc/8f3238f898163f09726c3d2b2cc9bafb09da26f3/library/core/src/future/mod.rs:84:19
  16: async_task::raw::RawTask::run
             at home/.cargo/registry/src/github.com-1ecc6299db9ec823/async-task-4.0.3/src/raw.rs:489:20
  17: async_task::runnable::Runnable::run
             at home/.cargo/registry/src/github.com-1ecc6299db9ec823/async-task-4.0.3/src/runnable.rs:309:18
  18: async_executor::Executor::try_tick
             at home/.cargo/registry/src/github.com-1ecc6299db9ec823/async-executor-1.4.1/src/lib.rs:181:17
  19: bevy_tasks::task_pool::TaskPool::scope::{{closure}}
             at home/.cargo/registry/src/github.com-1ecc6299db9ec823/bevy_tasks-0.5.0/src/task_pool.rs:222:21
  20: std::thread::local::LocalKey::try_with
             at /rustc/8f3238f898163f09726c3d2b2cc9bafb09da26f3/library/std/src/thread/local.rs:412:16
  21: std::thread::local::LocalKey::with
             at /rustc/8f3238f898163f09726c3d2b2cc9bafb09da26f3/library/std/src/thread/local.rs:388:9
  22: bevy_tasks::task_pool::TaskPool::scope
             at home/.cargo/registry/src/github.com-1ecc6299db9ec823/bevy_tasks-0.5.0/src/task_pool.rs:169:9
  23: ::run_systems
             at home/.cargo/registry/src/github.com-1ecc6299db9ec823/bevy_ecs-0.5.0/src/schedule/executor_parallel.rs:121:9
  24: ::run
             at home/.cargo/registry/src/github.com-1ecc6299db9ec823/bevy_ecs-0.5.0/src/schedule/stage.rs:822:17
  25: bevy_ecs::schedule::Schedule::run_once
             at home/.cargo/registry/src/github.com-1ecc6299db9ec823/bevy_ecs-0.5.0/src/schedule/mod.rs:201:13
  26: ::run
             at home/.cargo/registry/src/github.com-1ecc6299db9ec823/bevy_ecs-0.5.0/src/schedule/mod.rs:219:21
  27: bevy_app::app::App::update
             at home/.cargo/registry/src/github.com-1ecc6299db9ec823/bevy_app-0.5.0/src/app.rs:58:9
  28: mwm::main::{{closure}}
             at ./mwm/src/main.rs:14:13
  29:  as core::ops::function::Fn>::call
             at /rustc/8f3238f898163f09726c3d2b2cc9bafb09da26f3/library/alloc/src/boxed.rs:1825:9
  30: bevy_app::app::App::run
             at home/.cargo/registry/src/github.com-1ecc6299db9ec823/bevy_app-0.5.0/src/app.rs:68:9
  31: bevy_app::app_builder::AppBuilder::run
             at home/.cargo/registry/src/github.com-1ecc6299db9ec823/bevy_app-0.5.0/src/app_builder.rs:49:9
  32: mwm::main
             at ./mwm/src/main.rs:10:5
  33: core::ops::function::FnOnce::call_once
             at /rustc/8f3238f898163f09726c3d2b2cc9bafb09da26f3/library/core/src/ops/function.rs:227:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
pr2502 commented 2 years ago

Now looking at src/event.rs:resolve_event I'm pretty sure that it's not a x::StoreColors request, that the number matching is a coincidence. Instead this issue comes from the event being from a not-loaded (possibly even not compiled-in) extension, it is unfortunate that this causes a crash.

Would you be open to making the resolve_event functions fallible instead to allow users to handle events from disabled extensions appropriately for their application instead of crashing? I'd be happy to make a PR.

pr2502 commented 2 years ago

I have a simple workaround here https://github.com/pr2502/rust-xcb/commit/52a07402c136131860326fac57602b5ce70c28ac but the error should probably at least keep the suspect event code like the panic message had previously.

rtbo commented 2 years ago

Hi, a window manager is a regular client and has therefore access to the same API than any other client, so you will definitely not receive requests as events.

I see in your code that you do not activate any extension. For resolve_events to actually resolve an extension event, you have to activate the extensions you need by using Connection::connect_with_extensions or similar constructor. Can you try this?

The response_type == 89 indicates that this is the case. I would bet for randr because you seem to use it, and 89 is the base_event of randr on my system (it should be the same for you if your server have the same X extensions than mine). (xdpyinfo -queryExtensions will tell you)

I agree that there is no point panicking here. Instead of an error, I would rather go for an additional Event::Unknown that carries the original C event. It is semantically not incorrect, and I guess easier to use than an Error.

pr2502 commented 2 years ago

Oh thank you! I missed that I needed to enable the extensions when connecting.

I'll modify the patch for the unmatched events and make a PR for it.

pr2502 commented 2 years ago

fixed in #142