Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

wl_seat::get_pointer request issued on a seat without the pointer capability #83

Closed
st3r4g opened this issue Sep 16, 2019 · 10 comments
Closed

Comments

@st3r4g
Copy link

st3r4g commented Sep 16, 2019

From https://wayland.freedesktop.org/docs/html/apa.html#protocol-spec-wl_seat:

It is a protocol violation to issue this request on a seat that has never had the pointer capability.

Observed behaviour in alacritty and in the example clients (with WAYLAND_DEBUG=server):

[2601723.234] wl_registry@2.bind(4, "wl_seat", 5, new id [unknown]@27)
[2601723.259]  -> wl_seat@27.capabilities(2)
[2601723.267]  -> wl_seat@27.name("seat0")
[2601723.276] wl_compositor@3.create_surface(new id wl_surface@28)
[2601723.287] wl_seat@27.get_pointer(new id wl_pointer@29)
listener function for opcode 0 of wl_seat is NULL
thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Os { code: 32, kind: BrokenPipe, message: "Broken pipe" }', src/libcore/result.rs:999:5
note: Run with `RUST_BACKTRACE=1` environment variable to display a backtrace.

The bitfield value 2 sent by the server in the capability event means that the seat has keyboards but not pointer or touch devices. It should be easy to request the creation of the appropriate protocol objects based on the capabilities sent by the server.

@elinorbgr
Copy link
Member

Indeed, the Window::new_seat() method unconditionally binds the pointer, as such only seats with the pointer capability should be given to it. The examples (and I suspect winit) do not do that unfortunately.

@kchibisov
Copy link
Member

@st3r4g Would you mind running RUST_BACKTRACE=1 alacritty, so I can get a clue on what exactly kills it (there are two possible places, just checking)?

@st3r4g
Copy link
Author

st3r4g commented Sep 18, 2019

The crash happens if the compositor sets to NULL the implementation of wl_seat::get_pointer (a compositor could choose not to support pointers). Actually can be easily circumvented by implementing the request in the compositor as a no-op, but anyway the spec is pretty explicit that it is an error (and many other clients do not call it).

@kchibisov
Copy link
Member

Probably where is much clearer than what, since alacritty uses multiple libraries for Wayland things, knowing the exact line of unwrap will speed up things a lot.

@st3r4g
Copy link
Author

st3r4g commented Sep 18, 2019

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: Os { code: 32, kind: BrokenPipe, message: "Broken pipe" }', src/libcore/result.rs:999:5
stack backtrace:
   0: std::sys::unix::backtrace::tracing::imp::unwind_backtrace
             at src/libstd/sys/unix/backtrace/tracing/gcc_s.rs:39
   1: std::panicking::default_hook::{{closure}}
             at src/libstd/sys_common/backtrace.rs:71
             at src/libstd/sys_common/backtrace.rs:59
             at src/libstd/panicking.rs:197
   2: std::panicking::rust_panic_with_hook
             at src/libstd/panicking.rs:211
             at src/libstd/panicking.rs:474
   3: std::panicking::continue_panic_fmt
             at src/libstd/panicking.rs:381
   4: rust_begin_unwind
             at src/libstd/panicking.rs:308
   5: core::panicking::panic_fmt
             at src/libcore/panicking.rs:85
   6: core::result::unwrap_failed
             at /builddir/rustc-1.36.0-src/src/libcore/macros.rs:18
   7: winit::platform::platform::wayland::window::Window::new
             at /tmp/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/winit-0.19.1/src/platform/linux/wayland/window.rs:0
   8: alacritty_terminal::window::create_gl_window
             at /tmp/.cargo/registry/src/gh.neting.cc-1ecc6299db9ec823/winit-0.19.1/src/platform/linux/mod.rs:144
   9: alacritty_terminal::display::Display::new
             at alacritty_terminal/src/window.rs:158
  10: alacritty::main
             at alacritty/src/main.rs:142
             at alacritty/src/main.rs:114
  11: std::rt::lang_start::{{closure}}
             at /builddir/rustc-1.36.0-src/src/libstd/rt.rs:64
  12: main
  13: <unknown>
             at src/env/__libc_start_main.c:94

@kchibisov
Copy link
Member

I guess you've installed it from your distro package, since alacritty by default builds in release + debug symbols. You can follow alacritty's INSTALL.md instructions for your distro to build it in debug, also compile this PR instead of master.

P.s.
You should open bug on alacritty bug tracker, so we'll keep alacritty related discussion in alacritty repo.

@st3r4g
Copy link
Author

st3r4g commented Sep 18, 2019

I updated the backtrace. But why do you say this is an Alacritty bug? I thought the crash happened at the Rust libwayland implementation level as the compositor sets a function pointer to NULL and it gets called due to not following the protocol...

@elinorbgr
Copy link
Member

The crash happens if the compositor sets to NULL the implementation of wl_seat::get_pointer (a compositor could choose not to support pointers). Actually can be easily circumvented by implementing the request in the compositor as a no-op, but anyway the spec is pretty explicit that it is an error (and many other clients do not call it).

Note that setting NULL as the implementation of anything is a bug of the server though. A server should not crash due to ill-behaving clients, but rather detect protocol errors and kill the ill-behaving clients (with wl_resource_post_error).

In any case @kchibisov, both winit/SCTK and smithay-clipboard need to be fixed wrt to that.

For SCTK/winit, the issue is that Window::new_seat(..) unconditionally calls get_pointer. So we have two possibilities:

  • Change winit so that is ony calls Window::new_seat(..) with seats that actually have the pointer capability
  • Introduce some abstraction in SCTK to manage seats and handle this kind of questions automatically (this is quite related to SCTK API design reflexion #77)

@kchibisov
Copy link
Member

But why do you say this is an Alacritty bug?

The bug is not in alacritty, that's right, I just wanted for us(alacritty) make it easier to track things, we're already has a lot of dependency bugs on our bug tracker. Anyway I can remember it or open myself.

@vberger Yeah, I see. As I said before on IRC to you, I'll try to fix smithay-clipboard one, hopefully on this week.

@elinorbgr
Copy link
Member

This is fixed with the new refactor, as Window now automatically tracks seats without relying on outside help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants