Skip to content

Commit

Permalink
Add reentrant init test, implement initializing method on clack-host
Browse files Browse the repository at this point in the history
  • Loading branch information
prokopyl committed Apr 3, 2024
1 parent 0dd7c74 commit 8c42d1d
Show file tree
Hide file tree
Showing 5 changed files with 187 additions and 4 deletions.
2 changes: 1 addition & 1 deletion host/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ libloading = ["dep:libloading", "dep:stable_deref_trait"]

[dev-dependencies]
clack-plugin = { workspace = true }
clack-extensions = { workspace = true, features = ["clack-host", "latency", "log"] }
clack-extensions = { workspace = true, features = ["clack-host", "latency", "log", "timer"] }
clack-test-host = { workspace = true }

# nih_plug = { git = "https://github.com/robbert-vdh/nih-plug", features = ["assert_process_allocs"] }
Expand Down
47 changes: 44 additions & 3 deletions host/src/extensions/wrapper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@ use clap_sys::plugin::clap_plugin;
use std::panic::AssertUnwindSafe;
use std::pin::Pin;
use std::ptr::NonNull;
use std::sync::Arc;
use std::sync::atomic::{AtomicBool, Ordering};
use std::sync::{Arc, Once, OnceLock};

mod panic {
#[cfg(not(test))]
Expand All @@ -32,6 +33,11 @@ pub struct HostWrapper<H: Host> {
audio_processor: UnsafeOptionCell<<H as Host>::AudioProcessor<'static>>,
main_thread: UnsafeOptionCell<<H as Host>::MainThread<'static>>,
shared: Pin<Box<<H as Host>::Shared<'static>>>,

// Init stuff
init_guard: Once,
init_started: AtomicBool,
plugin_ptr: OnceLock<NonNull<clap_plugin>>,
}

// SAFETY: The only non-thread-safe methods on this type are unsafe
Expand All @@ -55,7 +61,14 @@ impl<H: Host> HostWrapper<H> {
where
F: FnOnce(&HostWrapper<H>) -> Result<T, HostWrapperError>,
{
match Self::from_raw(host).and_then(|h| Self::handle_panic(handler, h)) {
let result = Self::from_raw(host).and_then(|h| {
Self::handle_panic(h, |h| {
h.ensure_initializing_called();
handler(h)
})
});

match result {
Ok(value) => Some(value),
Err(_e) => {
// logging::plugin_log::<P>(host, &e); TODO
Expand Down Expand Up @@ -117,6 +130,9 @@ impl<H: Host> HostWrapper<H> {
audio_processor: UnsafeOptionCell::new(),
main_thread: UnsafeOptionCell::new(),
shared: Box::pin(shared(&())),
init_guard: Once::new(),
init_started: AtomicBool::new(false),
plugin_ptr: OnceLock::new(),
});

// PANIC: we have the only Arc copy of this wrapper data.
Expand All @@ -134,10 +150,16 @@ impl<H: Host> HostWrapper<H> {
unsafe { Pin::new_unchecked(wrapper) }
}

pub(crate) fn created(&self, instance: NonNull<clap_plugin>) {
let _ = self.plugin_ptr.set(instance);
}

/// # Safety
/// This must only be called on the main thread. User must ensure the provided instance pointer
/// is valid.
pub(crate) unsafe fn instantiated(&self, instance: NonNull<clap_plugin>) {
self.ensure_initializing_called();

// SAFETY: At this point there is no way main_thread could not have been set.
self.main_thread()
.as_mut()
Expand Down Expand Up @@ -209,7 +231,7 @@ impl<H: Host> HostWrapper<H> {
self.audio_processor.is_some()
}

fn handle_panic<T, F, Pa>(handler: F, param: Pa) -> Result<T, HostWrapperError>
fn handle_panic<T, F, Pa>(param: Pa, handler: F) -> Result<T, HostWrapperError>
where
F: FnOnce(Pa) -> Result<T, HostWrapperError>,
{
Expand All @@ -227,6 +249,25 @@ impl<H: Host> HostWrapper<H> {
.as_ref()
.ok_or(HostWrapperError::NullHostData)
}

fn ensure_initializing_called(&self) {
// This can only happen if the plugin tried to call a host method before init().
let Some(ptr) = self.plugin_ptr.get().copied() else {
return;
};

self.init_guard.call_once_force(|_| {
let result =
self.init_started
.compare_exchange(false, true, Ordering::SeqCst, Ordering::SeqCst);

// The comparison succeeded, and false was indeed the bool's value
if result == Ok(false) {
let handle = unsafe { PluginInitializingHandle::new(ptr) };

Check failure on line 266 in host/src/extensions/wrapper.rs

View workflow job for this annotation

GitHub Actions / clippy

unsafe block missing a safety comment

error: unsafe block missing a safety comment --> host/src/extensions/wrapper.rs:266:30 | 266 | let handle = unsafe { PluginInitializingHandle::new(ptr) }; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ | = help: consider adding a safety comment on the preceding line = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#undocumented_unsafe_blocks note: the lint level is defined here --> host/src/lib.rs:2:9 | 2 | #![deny(clippy::undocumented_unsafe_blocks)] | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
self.shared().initializing(handle);
}
})
}
}

#[derive(Copy, Clone, Eq, PartialEq, Debug)]
Expand Down
2 changes: 2 additions & 0 deletions host/src/plugin/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,8 @@ impl<H: Host> PluginInstanceInner<H> {
let plugin_instance_ptr =
unsafe { plugin_factory.create_plugin(plugin_id, raw_descriptor)? };

instance.host_wrapper.created(plugin_instance_ptr);

// Now instantiate the plugin
// SAFETY: TODO
unsafe {
Expand Down
139 changes: 139 additions & 0 deletions host/tests/reentrant-init.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,139 @@
use clack_extensions::timer::{
HostTimer, HostTimerImpl, PluginTimer, PluginTimerImpl, TimerError, TimerId,
};
use clack_host::prelude::*;
use clack_plugin::clack_entry;
use clack_plugin::prelude::*;
use std::sync::OnceLock;

struct MyPlugin;

impl Plugin for MyPlugin {
type AudioProcessor<'a> = ();
type Shared<'a> = ();
type MainThread<'a> = MyPluginMainThread;

fn declare_extensions(builder: &mut PluginExtensions<Self>, shared: Option<&Self::Shared<'_>>) {
assert!(shared.is_none()); // Host will only query extensions from within.
builder.register::<PluginTimer>();
}
}

struct MyPluginMainThread;

impl<'a> PluginMainThread<'a, ()> for MyPluginMainThread {}

impl PluginTimerImpl for MyPluginMainThread {
fn on_timer(&mut self, timer_id: TimerId) {
assert_eq!(timer_id, TimerId(5));
}
}

impl DefaultPluginFactory for MyPlugin {
fn get_descriptor() -> PluginDescriptor {
PluginDescriptor::new("my.plugin", "My plugin")
}

fn new_shared(_host: HostHandle) -> Result<Self::Shared<'_>, PluginError> {
Ok(())
}

fn new_main_thread(
mut host: HostMainThreadHandle,
_shared: &(),
) -> Result<MyPluginMainThread, PluginError> {
let timer: &HostTimer = host.shared().extension().unwrap();
let timer_id = timer.register_timer(&mut host, 1_000).unwrap();
assert_eq!(timer_id, TimerId(5));
Ok(MyPluginMainThread)
}
}

static MY_PLUGIN_ENTRY: EntryDescriptor = clack_entry!(SinglePluginEntry<MyPlugin>);

struct MyHost;

impl Host for MyHost {
type Shared<'a> = MyHostShared<'a>;
type MainThread<'a> = MyHostMainThread<'a>;
type AudioProcessor<'a> = ();

fn declare_extensions(builder: &mut HostExtensions<Self>, _shared: &Self::Shared<'_>) {
builder.register::<HostTimer>();
}
}

struct MyHostShared<'a> {
init: OnceLock<PluginInitializingHandle<'a>>,
}

impl<'a> HostShared<'a> for MyHostShared<'a> {
fn initializing(&self, instance: PluginInitializingHandle<'a>) {
let _ = self.init.set(instance);
}

fn request_restart(&self) {
unimplemented!()
}
fn request_process(&self) {
unimplemented!()
}
fn request_callback(&self) {
unimplemented!()
}
}

struct MyHostMainThread<'a> {
shared: &'a MyHostShared<'a>,
timer_registered: bool,
}

impl<'a> HostMainThread<'a> for MyHostMainThread<'a> {
fn instantiated(&mut self, _instance: PluginMainThreadHandle<'a>) {}
}

impl<'a> HostTimerImpl for MyHostMainThread<'a> {
fn register_timer(&mut self, period_ms: u32) -> Result<TimerId, TimerError> {
assert_eq!(period_ms, 1000);

let handle = self
.shared
.init
.get()
.expect("Initializing should have been called already!");

handle
.get_extension::<PluginTimer>()
.expect("Plugin should implement Timer extension!");

self.timer_registered = true;
Ok(TimerId(5))
}

fn unregister_timer(&mut self, _timer_id: TimerId) -> Result<(), TimerError> {
unimplemented!()
}
}

#[test]
fn can_call_host_methods_during_init() {
let host = HostInfo::new("host", "host", "host", "1.0").unwrap();

let bundle = unsafe { PluginBundle::load_from_raw(&MY_PLUGIN_ENTRY, "/my/plugin") }.unwrap();
let instance = PluginInstance::<MyHost>::new(
|_| MyHostShared {
init: OnceLock::new(),
},
|shared| MyHostMainThread {
shared,
timer_registered: false,
},
&bundle,
c"my.plugin",
&host,
)
.unwrap();

// Timer should have already been registered by the plugin during init().
assert!(instance.main_thread_host_data().timer_registered);
}
1 change: 1 addition & 0 deletions plugin/src/plugin/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ impl<'a, P: Plugin> PluginBoxInner<'a, P> {
unreachable!()
};

// FIXME: we can't use &mut on data as soon as init runs anymore.
match initializer.init(data.host.as_main_thread_unchecked()) {
Ok(wrapper) => {
data.plugin_data = Initialized(wrapper);
Expand Down

0 comments on commit 8c42d1d

Please sign in to comment.