helgoboss / reaper-rs

Rust bindings for the REAPER C++ API
MIT License
80 stars 8 forks source link

Should `ReaperSession` be `Send`? #42

Closed Boscop closed 2 years ago

Boscop commented 3 years ago

After updating to the most recent vst-rs (enabled by https://github.com/helgoboss/reaper-rs/issues/14), Plugin must be Send now. So I get this error:

image

What's the recommended fix? :)

helgoboss commented 3 years ago

Mmh, rather not. ReaperSession is something that should be created in the main thread and stay in the main thread. Why would you send it between threads?

Boscop commented 3 years ago

I just need to keep it alive to prevent my control surface from being dropped.

helgoboss commented 3 years ago

If this *mut c_void in PluginContext is the only reason why it doesn't implement Send, I guess we could make it Send. The void pointers are not like Rc in that they have some single-threaded reference counting or similar. And apart from it, making it Send wouldn't make things more unsafe than they already are anyway. If you use main-thread-only REAPER functions in another thread than in the one in which you created ReaperSession, reaper-rs will panic at runtime.

Do you want to create a PR?

Boscop commented 3 years ago

I would. But it's not just this one pointer:

error: src\lib.rs:124: `*mut std::ffi::c_void` cannot be sent between threads safely
error: src\lib.rs:124: `*mut std::ffi::c_void` cannot be sent between threads safely
help: src\lib.rs:124: within `MyPlugin`, the trait `std::marker::Send` is not implemented for `*mut std::ffi::c_void`
note: src\lib.rs:124: required because it appears within the type `reaper_low::PluginContext`
note: src\lib.rs:124: required because it appears within the type `std::option::Option<reaper_low::PluginContext>`
note: src\lib.rs:124: required because it appears within the type `reaper_low::Reaper`
note: src\lib.rs:124: required because it appears within the type `reaper_medium::Reaper`
note: src\lib.rs:124: required because it appears within the type `ReaperSession`
note: src\lib.rs:124: required because it appears within the type `ReaperData`
note: src\lib.rs:124: required because it appears within the type `std::option::Option<ReaperData>`
note: src\lib.rs:124: required because it appears within the type `MyPlugin`
error: src\lib.rs:1: required by a bound in this
error: C:\Users\me\.cargo\git\checkouts\vst-rs-7c987c17e49fdc9b\fd8e173\src\plugin.rs:473: required by this bound in `vst::plugin::Plugin`
error: src\lib.rs:124: `NonNull<std::ffi::c_void>` cannot be sent between threads safely
error: src\lib.rs:124: `NonNull<std::ffi::c_void>` cannot be sent between threads safely
help: src\lib.rs:124: within `(NonNull<std::ffi::c_void>, Box<Box<(dyn reaper_low::IReaperControlSurface + 'static)>>)`, the trait `std::marker::Send` is not implemented for `NonNull<std::ffi::c_void>`
note: src\lib.rs:124: required because it appears within the type `(NonNull<std::ffi::c_void>, Box<Box<(dyn reaper_low::IReaperControlSurface + 'static)>>)`
note: src\lib.rs:124: required because of the requirements on the impl of `std::marker::Send` for `hashbrown::raw::RawTable<(NonNull<std::ffi::c_void>, Box<Box<(dyn reaper_low::IReaperControlSurface + 'static)>>)>`
note: src\lib.rs:124: required because it appears within the type `hashbrown::map::HashMap<NonNull<std::ffi::c_void>, Box<Box<(dyn reaper_low::IReaperControlSurface + 'static)>>, RandomState>`
note: src\lib.rs:124: required because it appears within the type `HashMap<NonNull<std::ffi::c_void>, Box<Box<(dyn reaper_low::IReaperControlSurface + 'static)>>>`
note: src\lib.rs:124: required because it appears within the type `ReaperSession`
note: src\lib.rs:124: required because it appears within the type `ReaperData`
note: src\lib.rs:124: required because it appears within the type `std::option::Option<ReaperData>`
note: src\lib.rs:124: required because it appears within the type `MyPlugin`
error: src\lib.rs:1: required by a bound in this
error: C:\Users\me\.cargo\git\checkouts\vst-rs-7c987c17e49fdc9b\fd8e173\src\plugin.rs:473: required by this bound in `vst::plugin::Plugin`
error: src\lib.rs:124: `NonNull<gaccel_register_t>` cannot be sent between threads safely
error: src\lib.rs:124: `NonNull<gaccel_register_t>` cannot be sent between threads safely
help: src\lib.rs:124: within `(NonNull<gaccel_register_t>, Box<OwnedGaccelRegister>)`, the trait `std::marker::Send` is not implemented for `NonNull<gaccel_register_t>`
note: src\lib.rs:124: required because it appears within the type `(NonNull<gaccel_register_t>, Box<OwnedGaccelRegister>)`
note: src\lib.rs:124: required because of the requirements on the impl of `std::marker::Send` for `hashbrown::raw::RawTable<(NonNull<gaccel_register_t>, Box<OwnedGaccelRegister>)>`
note: src\lib.rs:124: required because it appears within the type `hashbrown::map::HashMap<NonNull<gaccel_register_t>, Box<OwnedGaccelRegister>, RandomState>`
note: src\lib.rs:124: required because it appears within the type `HashMap<NonNull<gaccel_register_t>, Box<OwnedGaccelRegister>>`
note: src\lib.rs:124: required because it appears within the type `reaper_medium::infostruct_keeper::InfostructKeeper<OwnedGaccelRegister, gaccel_register_t>`
note: src\lib.rs:124: required because it appears within the type `ReaperSession`
note: src\lib.rs:124: required because it appears within the type `ReaperData`
note: src\lib.rs:124: required because it appears within the type `std::option::Option<ReaperData>`
note: src\lib.rs:124: required because it appears within the type `MyPlugin`
error: src\lib.rs:1: required by a bound in this
error: C:\Users\me\.cargo\git\checkouts\vst-rs-7c987c17e49fdc9b\fd8e173\src\plugin.rs:473: required by this bound in `vst::plugin::Plugin`
error: src\lib.rs:124: `NonNull<audio_hook_register_t>` cannot be sent between threads safely
error: src\lib.rs:124: `NonNull<audio_hook_register_t>` cannot be sent between threads safely
help: src\lib.rs:124: within `(NonNull<audio_hook_register_t>, Box<reaper_medium::audio_hook_register::OwnedAudioHookRegister>)`, the trait `std::marker::Send` is not implemented for `NonNull<audio_hook_register_t>`
note: src\lib.rs:124: required because it appears within the type `(NonNull<audio_hook_register_t>, Box<reaper_medium::audio_hook_register::OwnedAudioHookRegister>)`
note: src\lib.rs:124: required because of the requirements on the impl of `std::marker::Send` for `hashbrown::raw::RawTable<(NonNull<audio_hook_register_t>, Box<reaper_medium::audio_hook_register::OwnedAudioHookRegister>)>`
note: src\lib.rs:124: required because it appears within the type `hashbrown::map::HashMap<NonNull<audio_hook_register_t>, Box<reaper_medium::audio_hook_register::OwnedAudioHookRegister>, RandomState>`
note: src\lib.rs:124: required because it appears within the type `HashMap<NonNull<audio_hook_register_t>, Box<reaper_medium::audio_hook_register::OwnedAudioHookRegister>>`
note: src\lib.rs:124: required because it appears within the type `reaper_medium::infostruct_keeper::InfostructKeeper<reaper_medium::audio_hook_register::OwnedAudioHookRegister, audio_hook_register_t>`
note: src\lib.rs:124: required because it appears within the type `ReaperSession`
note: src\lib.rs:124: required because it appears within the type `ReaperData`
note: src\lib.rs:124: required because it appears within the type `std::option::Option<ReaperData>`
note: src\lib.rs:124: required because it appears within the type `MyPlugin`
error: src\lib.rs:1: required by a bound in this
error: C:\Users\me\.cargo\git\checkouts\vst-rs-7c987c17e49fdc9b\fd8e173\src\plugin.rs:473: required by this bound in `vst::plugin::Plugin`
error: src\lib.rs:124: `*mut HWND__` cannot be sent between threads safely
error: src\lib.rs:124: `*mut HWND__` cannot be sent between threads safely
help: src\lib.rs:124: within `MyPlugin`, the trait `std::marker::Send` is not implemented for `*mut HWND__`
note: src\lib.rs:124: required because it appears within the type `reaper_low::ExtensionPluginContext`
note: src\lib.rs:124: required because it appears within the type `reaper_low::TypeSpecificPluginContext`
note: src\lib.rs:124: required because it appears within the type `reaper_low::PluginContext`
note: src\lib.rs:124: required because it appears within the type `std::option::Option<reaper_low::PluginContext>`
note: src\lib.rs:124: required because it appears within the type `reaper_low::Reaper`
note: src\lib.rs:124: required because it appears within the type `reaper_medium::Reaper`
note: src\lib.rs:124: required because it appears within the type `ReaperSession`
note: src\lib.rs:124: required because it appears within the type `ReaperData`
note: src\lib.rs:124: required because it appears within the type `std::option::Option<ReaperData>`
note: src\lib.rs:124: required because it appears within the type `MyPlugin`
error: src\lib.rs:1: required by a bound in this
error: C:\Users\me\.cargo\git\checkouts\vst-rs-7c987c17e49fdc9b\fd8e173\src\plugin.rs:473: required by this bound in `vst::plugin::Plugin`
error: src\lib.rs:124: `(dyn reaper_low::IReaperControlSurface + 'static)` cannot be sent between threads safely
error: src\lib.rs:124: `(dyn reaper_low::IReaperControlSurface + 'static)` cannot be sent between threads safely
help: src\lib.rs:124: the trait `std::marker::Send` is not implemented for `(dyn reaper_low::IReaperControlSurface + 'static)`
note: src\lib.rs:124: required because of the requirements on the impl of `std::marker::Send` for `std::ptr::Unique<(dyn reaper_low::IReaperControlSurface + 'static)>`
note: src\lib.rs:124: required because it appears within the type `Box<(dyn reaper_low::IReaperControlSurface + 'static)>`
note: src\lib.rs:124: required because of the requirements on the impl of `std::marker::Send` for `std::ptr::Unique<Box<(dyn reaper_low::IReaperControlSurface + 'static)>>`
note: src\lib.rs:124: required because it appears within the type `Box<Box<(dyn reaper_low::IReaperControlSurface + 'static)>>`
note: src\lib.rs:124: required because it appears within the type `(NonNull<std::ffi::c_void>, Box<Box<(dyn reaper_low::IReaperControlSurface + 'static)>>)`
note: src\lib.rs:124: required because of the requirements on the impl of `std::marker::Send` for `hashbrown::raw::RawTable<(NonNull<std::ffi::c_void>, Box<Box<(dyn reaper_low::IReaperControlSurface + 'static)>>)>`
note: src\lib.rs:124: required because it appears within the type `hashbrown::map::HashMap<NonNull<std::ffi::c_void>, Box<Box<(dyn reaper_low::IReaperControlSurface + 'static)>>, RandomState>`
note: src\lib.rs:124: required because it appears within the type `HashMap<NonNull<std::ffi::c_void>, Box<Box<(dyn reaper_low::IReaperControlSurface + 'static)>>>`
note: src\lib.rs:124: required because it appears within the type `ReaperSession`
note: src\lib.rs:124: required because it appears within the type `ReaperData`
note: src\lib.rs:124: required because it appears within the type `std::option::Option<ReaperData>`
note: src\lib.rs:124: required because it appears within the type `MyPlugin`
error: src\lib.rs:1: required by a bound in this
error: C:\Users\me\.cargo\git\checkouts\vst-rs-7c987c17e49fdc9b\fd8e173\src\plugin.rs:473: required by this bound in `vst::plugin::Plugin`
error: src\lib.rs:124: `*const i8` cannot be sent between threads safely
error: src\lib.rs:124: `*const i8` cannot be sent between threads safely
help: src\lib.rs:124: within `OwnedGaccelRegister`, the trait `std::marker::Send` is not implemented for `*const i8`
note: src\lib.rs:124: required because it appears within the type `gaccel_register_t`
note: src\lib.rs:124: required because it appears within the type `OwnedGaccelRegister`
note: src\lib.rs:124: required because of the requirements on the impl of `std::marker::Send` for `std::ptr::Unique<OwnedGaccelRegister>`
note: src\lib.rs:124: required because it appears within the type `Box<OwnedGaccelRegister>`
note: src\lib.rs:124: required because it appears within the type `(NonNull<gaccel_register_t>, Box<OwnedGaccelRegister>)`
note: src\lib.rs:124: required because of the requirements on the impl of `std::marker::Send` for `hashbrown::raw::RawTable<(NonNull<gaccel_register_t>, Box<OwnedGaccelRegister>)>`
note: src\lib.rs:124: required because it appears within the type `hashbrown::map::HashMap<NonNull<gaccel_register_t>, Box<OwnedGaccelRegister>, RandomState>`
note: src\lib.rs:124: required because it appears within the type `HashMap<NonNull<gaccel_register_t>, Box<OwnedGaccelRegister>>`
note: src\lib.rs:124: required because it appears within the type `reaper_medium::infostruct_keeper::InfostructKeeper<OwnedGaccelRegister, gaccel_register_t>`
note: src\lib.rs:124: required because it appears within the type `ReaperSession`
note: src\lib.rs:124: required because it appears within the type `ReaperData`
note: src\lib.rs:124: required because it appears within the type `std::option::Option<ReaperData>`
note: src\lib.rs:124: required because it appears within the type `MyPlugin`
error: src\lib.rs:1: required by a bound in this
error: C:\Users\me\.cargo\git\checkouts\vst-rs-7c987c17e49fdc9b\fd8e173\src\plugin.rs:473: required by this bound in `vst::plugin::Plugin`
error: src\lib.rs:124: `(dyn OnAudioBuffer + 'static)` cannot be sent between threads safely
error: src\lib.rs:124: `(dyn OnAudioBuffer + 'static)` cannot be sent between threads safely
help: src\lib.rs:124: the trait `std::marker::Send` is not implemented for `(dyn OnAudioBuffer + 'static)`
note: src\lib.rs:124: required because of the requirements on the impl of `std::marker::Send` for `std::ptr::Unique<(dyn OnAudioBuffer + 'static)>`
note: src\lib.rs:124: required because it appears within the type `Box<(dyn OnAudioBuffer + 'static)>`
note: src\lib.rs:124: required because it appears within the type `reaper_medium::audio_hook_register::OwnedAudioHookRegister`
note: src\lib.rs:124: required because of the requirements on the impl of `std::marker::Send` for `std::ptr::Unique<reaper_medium::audio_hook_register::OwnedAudioHookRegister>`
note: src\lib.rs:124: required because it appears within the type `Box<reaper_medium::audio_hook_register::OwnedAudioHookRegister>`
note: src\lib.rs:124: required because it appears within the type `(NonNull<audio_hook_register_t>, Box<reaper_medium::audio_hook_register::OwnedAudioHookRegister>)`
note: src\lib.rs:124: required because of the requirements on the impl of `std::marker::Send` for `hashbrown::raw::RawTable<(NonNull<audio_hook_register_t>, Box<reaper_medium::audio_hook_register::OwnedAudioHookRegister>)>`
note: src\lib.rs:124: required because it appears within the type `hashbrown::map::HashMap<NonNull<audio_hook_register_t>, Box<reaper_medium::audio_hook_register::OwnedAudioHookRegister>, RandomState>`
note: src\lib.rs:124: required because it appears within the type `HashMap<NonNull<audio_hook_register_t>, Box<reaper_medium::audio_hook_register::OwnedAudioHookRegister>>`
note: src\lib.rs:124: required because it appears within the type `reaper_medium::infostruct_keeper::InfostructKeeper<reaper_medium::audio_hook_register::OwnedAudioHookRegister, audio_hook_register_t>`
note: src\lib.rs:124: required because it appears within the type `ReaperSession`
note: src\lib.rs:124: required because it appears within the type `ReaperData`
note: src\lib.rs:124: required because it appears within the type `std::option::Option<ReaperData>`
note: src\lib.rs:124: required because it appears within the type `MyPlugin`
error: src\lib.rs:1: required by a bound in this
error: C:\Users\me\.cargo\git\checkouts\vst-rs-7c987c17e49fdc9b\fd8e173\src\plugin.rs:473: required by this bound in `vst::plugin::Plugin`

Is it safe (i.e. not more unsafe than it already is) to make all necessary types Send?

helgoboss commented 3 years ago

I think yes. This all boils down to the same thing: Raw pointers. And that OnAudioBuffer trait object - which I think should really have a Send bound anyway because it needs to be sent from the main thread to the audio thread to make sense.

But I can't tell for sure. This would need some thinking time to check thoroughly.

For the moment you could implement Send unsafely for your plug-in and see how that goes.

helgoboss commented 2 years ago

I had to make ReaperSession and a bunch of other things Send now anyway because of the requirements of the new vst crate version. Having looked into it a bit closer, I'm pretty sure those raw pointers don't do any harm when being sent to another thread.