Smithay / client-toolkit

Smithay's toolkit for writing wayland clients
MIT License
270 stars 77 forks source link

Crash when calling window.refresh() from a different thread #80

Closed YaLTeR closed 4 years ago

YaLTeR commented 5 years ago

Example: take kbd_input.rs and replace the event loop with the following:

    let window = Arc::new(Mutex::new(window));
    {
        let window = window.clone();
        std::thread::spawn(move || {
            loop {
                window.lock().unwrap().refresh();
                std::thread::sleep_ms(100);
            }
        });
    }

    loop {
        event_queue.dispatch().unwrap();
    }

Crash:

thread '<unnamed>' panicked at 'called `Option::unwrap()` on a `None` value', src/libcore/option.rs:347:21
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
stack backtrace:
   0: std::sys::unix::backtrace::tracing::imp::unwind_backtrace
             at src/libstd/sys/unix/backtrace/tracing/gcc_s.rs:39
   1: std::sys_common::backtrace::_print
             at src/libstd/sys_common/backtrace.rs:71
   2: std::panicking::default_hook::{{closure}}
             at src/libstd/sys_common/backtrace.rs:59
             at src/libstd/panicking.rs:197
   3: std::panicking::default_hook
             at src/libstd/panicking.rs:211
   4: std::panicking::rust_panic_with_hook
             at src/libstd/panicking.rs:474
   5: std::panicking::continue_panic_fmt
             at src/libstd/panicking.rs:381
   6: rust_begin_unwind
             at src/libstd/panicking.rs:308
   7: core::panicking::panic_fmt
             at src/libcore/panicking.rs:85
   8: core::panicking::panic
             at src/libcore/panicking.rs:49
   9: core::option::Option<T>::unwrap
             at /rustc/a53f9df32fbb0b5f4382caaad8f1a46f36ea887c/src/libcore/macros.rs:12
  10: <smithay_client_toolkit::window::concept_frame::ConceptFrame as smithay_client_toolkit::window::Frame>::redraw::{{closure}}
             at src/window/concept_frame.rs:468
  11: core::ops::function::impls::<impl core::ops::function::FnOnce<A> for &mut F>::call_once
             at /rustc/a53f9df32fbb0b5f4382caaad8f1a46f36ea887c/src/libcore/ops/function.rs:279
  12: core::option::Option<T>::map
             at /rustc/a53f9df32fbb0b5f4382caaad8f1a46f36ea887c/src/libcore/option.rs:416
  13: <core::iter::adapters::Map<I,F> as core::iter::traits::iterator::Iterator>::next
             at /rustc/a53f9df32fbb0b5f4382caaad8f1a46f36ea887c/src/libcore/iter/adapters/mod.rs:570
  14: <core::iter::adapters::flatten::FlattenCompat<I,U> as core::iter::traits::iterator::Iterator>::next
             at /rustc/a53f9df32fbb0b5f4382caaad8f1a46f36ea887c/src/libcore/iter/adapters/flatten.rs:219
  15: <core::iter::adapters::flatten::FlatMap<I,U,F> as core::iter::traits::iterator::Iterator>::next
             at /rustc/a53f9df32fbb0b5f4382caaad8f1a46f36ea887c/src/libcore/iter/adapters/flatten.rs:49
  16: <alloc::vec::Vec<T> as alloc::vec::SpecExtend<T,I>>::from_iter
             at /rustc/a53f9df32fbb0b5f4382caaad8f1a46f36ea887c/src/liballoc/vec.rs:1819
  17: <alloc::vec::Vec<T> as core::iter::traits::collect::FromIterator<T>>::from_iter
             at /rustc/a53f9df32fbb0b5f4382caaad8f1a46f36ea887c/src/liballoc/vec.rs:1731
  18: core::iter::traits::iterator::Iterator::collect
             at /rustc/a53f9df32fbb0b5f4382caaad8f1a46f36ea887c/src/libcore/iter/traits/iterator.rs:1465
  19: <smithay_client_toolkit::window::concept_frame::ConceptFrame as smithay_client_toolkit::window::Frame>::redraw
             at src/window/concept_frame.rs:462
  20: smithay_client_toolkit::window::Window<F>::refresh
             at ./src/window/mod.rs:406
  21: crash::main::{{closure}}
             at examples/crash.rs:125
elinorbgr commented 4 years ago

The core of the issue is that ThemeManager::theme_pointer_with_impl() does not register the implementation as threadsafe, and as such wayland-client forbids access to the user data from an other thread.

There won't be a satisfying way to fix it API-wise without updating to wayland-client 0.24, which decouples the questions of thread-safety of the implementation & user data.

elinorbgr commented 4 years ago

Following the lib refactor, the Window can no longer be shared across threads, due to most of its inner workings requiring the creation of new wayland objects, which cannot be safely done from an other thread than the one hosting the event queue.