RustAudio / baseview

low-level window system interface for audio plugin UIs
Apache License 2.0
267 stars 57 forks source link

Panic in Reaper on Linux when closing plugin window #142

Open ilmai opened 1 year ago

ilmai commented 1 year ago

Tested on two systems. I get this crash inside baseview when closing the plugin window of my plugin using Vizia:

17:39:21 [ERROR] thread 'unnamed' panicked at 'called `Result::unwrap()` on an `Err` value: XLibError { error_code: 9, error_message: "BadDrawable (invalid Pixmap or Window parameter)", minor_code: 3, request_code: 130, type: 0, resource_id: 8388612, serial: 2718 }': /home/jussi/.cargo/git/checkouts/baseview-9d6c750431f4479e/7001c25/src/gl/x11.rs:246
   0: nih_plug::wrapper::util::log_panics::{{closure}}::{{closure}}
             at /home/jussi/.cargo/git/checkouts/nih-plug-a2d2dc277b128e13/ce2eab8/src/wrapper/util.rs:128:29
      nih_plug::util::permit_alloc
             at /home/jussi/.cargo/git/checkouts/nih-plug-a2d2dc277b128e13/ce2eab8/src/util.rs:25:5
   1: <alloc::boxed::Box<F,A> as core::ops::function::Fn<Args>>::call
             at /rustc/8460ca823e8367a30dda430efda790588b8c84d3/library/alloc/src/boxed.rs:2002:9
      std::panicking::rust_panic_with_hook
             at /rustc/8460ca823e8367a30dda430efda790588b8c84d3/library/std/src/panicking.rs:692:13
   2: std::panicking::begin_panic_handler::{{closure}}
             at /rustc/8460ca823e8367a30dda430efda790588b8c84d3/library/std/src/panicking.rs:579:13
   3: std::sys_common::backtrace::__rust_end_short_backtrace
             at /rustc/8460ca823e8367a30dda430efda790588b8c84d3/library/std/src/sys_common/backtrace.rs:137:18
   4: rust_begin_unwind
             at /rustc/8460ca823e8367a30dda430efda790588b8c84d3/library/std/src/panicking.rs:575:5
   5: core::panicking::panic_fmt
             at /rustc/8460ca823e8367a30dda430efda790588b8c84d3/library/core/src/panicking.rs:64:14
   6: core::result::unwrap_failed
             at /rustc/8460ca823e8367a30dda430efda790588b8c84d3/library/core/src/result.rs:1790:5
   7: baseview::gl::x11::GlContext::swap_buffers::{{closure}}
      baseview::gl::x11::errors::XErrorHandler::handle::{{closure}}::{{closure}}
             at /home/jussi/.cargo/git/checkouts/baseview-9d6c750431f4479e/7001c25/src/gl/x11/errors.rs:73:17
      <core::panic::unwind_safe::AssertUnwindSafe<F> as core::ops::function::FnOnce<()>>::call_once
             at /rustc/8460ca823e8367a30dda430efda790588b8c84d3/library/core/src/panic/unwind_safe.rs:271:9
      std::panicking::try::do_call
             at /rustc/8460ca823e8367a30dda430efda790588b8c84d3/library/std/src/panicking.rs:483:40
      std::panicking::try
             at /rustc/8460ca823e8367a30dda430efda790588b8c84d3/library/std/src/panicking.rs:447:19
      std::panic::catch_unwind
             at /rustc/8460ca823e8367a30dda430efda790588b8c84d3/library/std/src/panic.rs:140:14
      baseview::gl::x11::errors::XErrorHandler::handle::{{closure}}
             at /home/jussi/.cargo/git/checkouts/baseview-9d6c750431f4479e/7001c25/src/gl/x11/errors.rs:71:32
      std::thread::local::LocalKey<T>::try_with
             at /rustc/8460ca823e8367a30dda430efda790588b8c84d3/library/std/src/thread/local.rs:446:16
      std::thread::local::LocalKey<T>::with
             at /rustc/8460ca823e8367a30dda430efda790588b8c84d3/library/std/src/thread/local.rs:422:9
   8: baseview::gl::x11::errors::XErrorHandler::handle
             at /home/jussi/.cargo/git/checkouts/baseview-9d6c750431f4479e/7001c25/src/gl/x11/errors.rs:66:9
      baseview::gl::x11::GlContext::swap_buffers
             at /home/jussi/.cargo/git/checkouts/baseview-9d6c750431f4479e/7001c25/src/gl/x11.rs:242:9
      baseview::gl::GlContext::swap_buffers
             at /home/jussi/.cargo/git/checkouts/baseview-9d6c750431f4479e/7001c25/src/gl/mod.rs:107:9
   9: <vizia_baseview::window::ViziaWindow as baseview::window::WindowHandler>::on_frame
             at /home/jussi/.cargo/git/checkouts/vizia-629668219022e428/39e295a/crates/vizia_baseview/src/window.rs:221:9
  10: baseview::x11::window::Window::run_event_loop
             at /home/jussi/.cargo/git/checkouts/baseview-9d6c750431f4479e/7001c25/src/x11/window.rs:478:17
  11: baseview::x11::window::Window::window_thread
             at /home/jussi/.cargo/git/checkouts/baseview-9d6c750431f4479e/7001c25/src/x11/window.rs:365:9
  12: baseview::x11::window::Window::open_parented::{{closure}}
             at /home/jussi/.cargo/git/checkouts/baseview-9d6c750431f4479e/7001c25/src/x11/window.rs:140:13
      std::sys_common::backtrace::__rust_begin_short_backtrace
             at /rustc/8460ca823e8367a30dda430efda790588b8c84d3/library/std/src/sys_common/backtrace.rs:121:18
  13: std::thread::Builder::spawn_unchecked_::{{closure}}::{{closure}}
             at /rustc/8460ca823e8367a30dda430efda790588b8c84d3/library/std/src/thread/mod.rs:558:17
      <core::panic::unwind_safe::AssertUnwindSafe<F> as core::ops::function::FnOnce<()>>::call_once
             at /rustc/8460ca823e8367a30dda430efda790588b8c84d3/library/core/src/panic/unwind_safe.rs:271:9
      std::panicking::try::do_call
             at /rustc/8460ca823e8367a30dda430efda790588b8c84d3/library/std/src/panicking.rs:483:40
      std::panicking::try
             at /rustc/8460ca823e8367a30dda430efda790588b8c84d3/library/std/src/panicking.rs:447:19
      std::panic::catch_unwind
             at /rustc/8460ca823e8367a30dda430efda790588b8c84d3/library/std/src/panic.rs:140:14
      std::thread::Builder::spawn_unchecked_::{{closure}}
             at /rustc/8460ca823e8367a30dda430efda790588b8c84d3/library/std/src/thread/mod.rs:557:30
      core::ops::function::FnOnce::call_once{{vtable.shim}}
             at /rustc/8460ca823e8367a30dda430efda790588b8c84d3/library/core/src/ops/function.rs:250:5
  14: <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
             at /rustc/8460ca823e8367a30dda430efda790588b8c84d3/library/alloc/src/boxed.rs:1988:9
      <alloc::boxed::Box<F,A> as core::ops::function::FnOnce<Args>>::call_once
             at /rustc/8460ca823e8367a30dda430efda790588b8c84d3/library/alloc/src/boxed.rs:1988:9
      std::sys::unix::thread::Thread::new::thread_start
             at /rustc/8460ca823e8367a30dda430efda790588b8c84d3/library/std/src/sys/unix/thread.rs:108:17
  15: start_thread
             at ./nptl/pthread_create.c:442:8
  16: clone3
             at ./misc/../sysdeps/unix/sysv/linux/x86_64/clone3.S:81
PolyVector commented 2 months ago

I'm getting reports of the same issue from users, and can confirm it happens when using Vizia, Iced, and Egui, so it's not tied to Vizia specifically.

Edit: I should mention I reproduced the error using the nih-plug example plugins, and a Linux Mint (Debian Edition) VM. I've also seen it on regular Debian/Ubuntu VMs, but it was particularly quick to reproduce on the Mint (Debian Edition) one.

PolyVector commented 1 month ago

I've finally managed to track down the issue, and have a quick fix! It's caused by a race condition that starts in baseview::x11::WindowHandle::close().

Explaination of the Bug: WindowHandle::close() sets the close_requested atomic so that the event loop knows to finish up, but the function returns immediately allowing windows to close before baseview can complete rendering and process the close request. To make matters worse, this frequently happens at the start of frame rendering because activating an OpenGL context will wait for things to catch up (like vsync).

Because it's a race condition that also depends on driver quirks, every system I've tested behaves differently. Usually crashes occur upon reopening the window. Some systems crash on the first reopening, some can survive 100, it's all over the place.

A proper fix would be to synchronize with the event loop thread, as the existing comment points out... I wish I had spotted that before spending an embarrassing amount of time tracking this down. 🤦 A quick-and-dirty fix is to add a small delay when closing:

impl WindowHandle {
    pub fn close(&mut self) {
        if self.raw_window_handle.take().is_some() {
            // FIXME: This will need to be changed from just setting an atomic to somehow
            // synchronizing with the window being closed (using a synchronous channel, or
            // by joining on the event loop thread).

            self.close_requested.store(true, Ordering::Relaxed);

            // HACK: Allow the window time to close nicely to avoid OpenGL errors/crashing
            //       until proper synchronizing (mentioned above) can be implemented.
            thread::sleep(Duration::from_millis(200));
        }
    }

    ...

These issues are the same: https://github.com/robbert-vdh/nih-plug/issues/98 https://github.com/ardura/Subhoofer/issues/9 Possibly the same: https://github.com/robbert-vdh/nih-plug/issues/124