Skip to content

Commit

Permalink
Make PluginBundle::load unsafe
Browse files Browse the repository at this point in the history
  • Loading branch information
prokopyl committed Apr 7, 2024
1 parent ebe64ed commit b29d152
Show file tree
Hide file tree
Showing 5 changed files with 59 additions and 26 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ pub fn load_and_process() -> Result<(), Box<dyn std::error::Error>> {
// Information about our totally legit host.
let host_info = HostInfo::new("Legit Studio", "Legit Ltd.", "https://example.com", "4.3.2")?;

let bundle = PluginBundle::load("/home/user/.clap/u-he/libdiva.so")?;
let bundle = unsafe { PluginBundle::load("/home/user/.clap/u-he/libdiva.so")? };
let plugin_factory = bundle.get_plugin_factory().unwrap();

let plugin_descriptor = plugin_factory.plugin_descriptors()
Expand Down
9 changes: 6 additions & 3 deletions host/examples/cpal/src/discovery.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
// Discovering plugins means loading them, which is unsafe
#![allow(unsafe_code)]

use clack_host::bundle::PluginBundleError;
use clack_host::prelude::*;
use rayon::prelude::*;
Expand Down Expand Up @@ -126,7 +129,7 @@ fn scan_plugins(bundles: &[DirEntry], searched_id: &str) -> Vec<FoundBundlePlugi
/// Scans a given bundle, looking for a plugin matching the given ID.
/// If this file wasn't a bundle or doesn't contain a plugin with a given ID, this returns `None`.
fn scan_plugin(path: &Path, searched_id: &str) -> Option<FoundBundlePlugin> {
let Ok(bundle) = PluginBundle::load(path) else {
let Ok(bundle) = (unsafe { PluginBundle::load(path) }) else {
return None;
};
for plugin in bundle.get_plugin_factory()?.plugin_descriptors() {
Expand Down Expand Up @@ -195,7 +198,7 @@ impl Display for PluginDescriptor {
pub fn list_plugins_in_bundle(
bundle_path: &Path,
) -> Result<Vec<FoundBundlePlugin>, DiscoveryError> {
let bundle = PluginBundle::load(bundle_path)?;
let bundle = unsafe { PluginBundle::load(bundle_path)? };
let Some(plugin_factory) = bundle.get_plugin_factory() else {
return Err(DiscoveryError::MissingPluginFactory);
};
Expand Down Expand Up @@ -242,7 +245,7 @@ pub fn load_plugin_id_from_path(
bundle_path: &Path,
id: &str,
) -> Result<Option<FoundBundlePlugin>, DiscoveryError> {
let bundle = PluginBundle::load(bundle_path)?;
let bundle = unsafe { PluginBundle::load(bundle_path)? };
let Some(plugin_factory) = bundle.get_plugin_factory() else {
return Err(DiscoveryError::MissingPluginFactory);
};
Expand Down
66 changes: 49 additions & 17 deletions host/src/bundle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,22 @@
//!
//! See the [`PluginBundle`]'s type documentation for examples.
//!
//! # Safety
//!
//! All functions that produce [`PluginBundle`]s from a CLAP bundle file or pointer are inherently
//! unsafe.
//!
//! Most APIs in this crate operate under the assumption that bundles and plugins are compliant
//! to the CLAP specification, and using a number of those APIs can easily result in
//! Undefined Behavior if operating on non-compliant plugins.
//!
//! Therefore, the safe APIs in this crate are safeguarding the host implementation, not on the
//! plugins it loads. As soon as a plugin is loaded, Undefined Behavior is possible to be triggered,
//! regardless of the host's implementation.
//!
//! Users needing to safeguard against crashes or other kinds of UB from plugins from affecting the
//! rest of their application should consider using additional process isolation techniques.
//!
//! # Plugin bundle discovery
//!
//! As of now, Clack does not implement any utilities to aid host implementations with discovering
Expand Down Expand Up @@ -84,7 +100,7 @@ use clack_common::utils::ClapVersion;
/// # pub fn main() -> Result<(), Box<dyn std::error::Error>> {
/// use clack_host::prelude::PluginBundle;
///
/// let bundle = PluginBundle::load("/home/user/.clap/u-he/libdiva.so")?;
/// let bundle = unsafe { PluginBundle::load("/home/user/.clap/u-he/libdiva.so")? };
/// let plugin_factory = bundle.get_plugin_factory().unwrap();
///
/// println!("Loaded bundle CLAP version: {}", bundle.version());
Expand All @@ -98,6 +114,15 @@ pub struct PluginBundle {
impl PluginBundle {
/// Loads a CLAP bundle from a file located at the given path.
///
/// # Safety
///
/// This function loads an external library object file, which is inherently unsafe, as even
/// just loading it can trigger any behavior in your application, including Undefined Behavior.
///
/// Additionally, loading a non-compliant CLAP bundle may invalidate safety assumptions other
/// APIs in this library rely on. See the [module docs](self)'s Safety section for more
/// information.
///
/// # Errors
///
/// This method returns an error if loading the bundle fails.
Expand All @@ -109,13 +134,13 @@ impl PluginBundle {
/// # pub fn main() -> Result<(), Box<dyn std::error::Error>> {
/// use clack_host::prelude::PluginBundle;
///
/// let bundle = PluginBundle::load("/home/user/.clap/u-he/libdiva.so")?;
/// let bundle = unsafe { PluginBundle::load("/home/user/.clap/u-he/libdiva.so")? };
///
/// println!("Loaded bundle CLAP version: {}", bundle.version());
/// # Ok(()) }
/// ```
#[cfg(feature = "libloading")]
pub fn load<P: AsRef<std::ffi::OsStr>>(path: P) -> Result<Self, PluginBundleError> {
pub unsafe fn load<P: AsRef<std::ffi::OsStr>>(path: P) -> Result<Self, PluginBundleError> {
use crate::bundle::library::PluginEntryLibrary;

let path = path.as_ref();
Expand All @@ -131,6 +156,19 @@ impl PluginBundle {

/// Loads a CLAP bundle from a given symbol in a given [`libloading::Library`].
///
/// This function takes ownership of the [`libloading::Library`] object, ensuring it stays
/// properly loaded as long as the resulting [`PluginBundle`] or any plugin instance is kept
/// alive.
///
/// # Safety
///
/// The given path must match the file location the library was loaded from. Moreover, the
/// symbol named `symbol_name` must be a valid CLAP entry.
///
/// Additionally, loading a non-compliant CLAP bundle may invalidate safety assumptions other
/// APIs in this library rely on. See the [module docs](self)'s Safety section for more
/// information.
///
/// # Errors
///
/// This method returns an error if loading the bundle fails.
Expand All @@ -153,11 +191,6 @@ impl PluginBundle {
/// println!("Loaded bundle CLAP version: {}", bundle.version());
/// # Ok(()) }
/// ```
///
/// # Safety
///
/// The given path must match the file location the library was loaded from. Moreover, the
/// symbol named `symbol_name` must be a valid CLAP entry.
#[cfg(feature = "libloading")]
pub unsafe fn load_from_symbol_in_library<P: AsRef<std::ffi::OsStr>>(
path: P,
Expand All @@ -181,18 +214,17 @@ impl PluginBundle {
/// Note that CLAP plugins loaded this way still need a valid path, as they may perform various
/// filesystem operations relative to their bundle files.
///
/// # Safety
///
/// Loading a non-compliant CLAP bundle may invalidate safety assumptions other
/// APIs in this library rely on. See the [module docs](self)'s Safety section for more
/// information.
///
/// # Errors
///
/// This method returns an error if initializing the entry fails.
/// See [`PluginBundleError`] for all the possible errors that may occur.
///
/// # Safety
///
/// Users of this function *must* also ensure the [`EntryDescriptor`]'s fields are all
/// valid as per the
/// [CLAP specification](https://github.com/free-audio/clap/blob/main/include/clap/entry.h), as
/// any undefined behavior may otherwise occur.
///
/// # Example
///
/// ```no_run
Expand Down Expand Up @@ -239,7 +271,7 @@ impl PluginBundle {
/// use clack_host::factory::PluginFactory;
/// use clack_host::prelude::PluginBundle;
///
/// let bundle = PluginBundle::load("/home/user/.clap/u-he/libdiva.so")?;
/// let bundle = unsafe { PluginBundle::load("/home/user/.clap/u-he/libdiva.so")? };
/// let plugin_factory = bundle.get_factory::<PluginFactory>().unwrap();
///
/// println!("Found {} plugins.", plugin_factory.plugin_count());
Expand All @@ -265,7 +297,7 @@ impl PluginBundle {
/// # pub fn main() -> Result<(), Box<dyn std::error::Error>> {
/// use clack_host::prelude::PluginBundle;
///
/// let bundle = PluginBundle::load("/home/user/.clap/u-he/libdiva.so")?;
/// let bundle = unsafe { PluginBundle::load("/home/user/.clap/u-he/libdiva.so")? };
/// let plugin_factory = bundle.get_plugin_factory().unwrap();
///
/// println!("Found {} plugins.", plugin_factory.plugin_count());
Expand Down
4 changes: 1 addition & 3 deletions host/src/plugin/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ impl<H: HostHandlers> PluginInstanceInner<H> {
}

// Now instantiate the plugin
// SAFETY: TODO
// SAFETY: The CLAP spec requires those function pointers to be valid to call
unsafe {
if let Some(init) = plugin_instance_ptr.as_ref().init {
if !init(plugin_instance_ptr.as_ptr()) {
Expand Down Expand Up @@ -114,8 +114,6 @@ impl<H: HostHandlers> PluginInstanceInner<H> {
.activate
.ok_or(HostError::NullActivateFunction)?;

// FIXME: reentrancy if activate() calls audio_processor methods

// SAFETY: this method being &mut guarantees nothing can call any other main-thread method
unsafe {
self.host_wrapper.setup_audio_processor(audio_processor)?;
Expand Down
4 changes: 2 additions & 2 deletions host/tests/loading.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ pub fn it_works() {
std::env::consts::DLL_PREFIX,
std::env::consts::DLL_SUFFIX
);
let bundle = PluginBundle::load(bundle_path).unwrap();
let bundle = unsafe { PluginBundle::load(bundle_path).unwrap() };

let desc = bundle
.get_factory::<PluginFactory>()
Expand All @@ -33,7 +33,7 @@ pub fn it_works_concurrently() {
std::thread::scope(|s| {
for _ in 0..300 {
s.spawn(|| {
let bundle = PluginBundle::load(&bundle_path).unwrap();
let bundle = unsafe { PluginBundle::load(&bundle_path).unwrap() };

let desc = bundle
.get_factory::<PluginFactory>()
Expand Down

0 comments on commit b29d152

Please sign in to comment.