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

Panic in Reaper on Linux when closing plugin window #142

Open
Tracked by #168
ilmai opened this issue Mar 27, 2023 · 4 comments
Open
Tracked by #168

Panic in Reaper on Linux when closing plugin window #142

ilmai opened this issue Mar 27, 2023 · 4 comments

Comments

@ilmai
Copy link
Contributor

ilmai commented Mar 27, 2023

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
Copy link

PolyVector commented Jun 3, 2024

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
Copy link

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:
robbert-vdh/nih-plug#98
ardura/Subhoofer#9
Possibly the same:
robbert-vdh/nih-plug#124

@maxxatgit
Copy link

@PolyVector , did this quick fix get implemented? I would like to notify @ardura if it has... I'm waiting to use their plugins in Reaper.
-Thanks!

@PolyVector
Copy link

To my knowledge this bug hasn't been addressed yet in baseview unfortunately.

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