diff --git a/src/app/plugin_host.rs b/src/app/plugin_host.rs index 47c143779..9eabf3e82 100644 --- a/src/app/plugin_host.rs +++ b/src/app/plugin_host.rs @@ -226,23 +226,37 @@ impl crate::command::CadCommand for PluginInteractiveAdapter { fn name(&self) -> &'static str { "PLUGIN" } + // Every call into the plugin runs under a panic guard (#145): a buggy plugin + // that panics mid-command leaves the host running — the command just ends. fn prompt(&self) -> String { - self.inner.prompt() + crate::plugin::guard("InteractiveCommand::prompt", || self.inner.prompt()) + .unwrap_or_default() } fn on_point(&mut self, pt: glam::Vec3) -> crate::command::CmdResult { - plugin_step_to_result(self.inner.on_point([pt.x as f64, pt.y as f64, pt.z as f64])) + crate::plugin::guard("InteractiveCommand::on_point", || { + self.inner.on_point([pt.x as f64, pt.y as f64, pt.z as f64]) + }) + .map(plugin_step_to_result) + .unwrap_or(crate::command::CmdResult::Cancel) } fn on_enter(&mut self) -> crate::command::CmdResult { - plugin_step_to_result(self.inner.on_enter()) + crate::plugin::guard("InteractiveCommand::on_enter", || self.inner.on_enter()) + .map(plugin_step_to_result) + .unwrap_or(crate::command::CmdResult::Cancel) } fn needs_entity_pick(&self) -> bool { - self.inner.needs_object_pick() + crate::plugin::guard("InteractiveCommand::needs_object_pick", || { + self.inner.needs_object_pick() + }) + .unwrap_or(false) } fn on_entity_pick(&mut self, handle: Handle, pt: glam::Vec3) -> crate::command::CmdResult { - plugin_step_to_result( + crate::plugin::guard("InteractiveCommand::on_object_pick", || { self.inner - .on_object_pick(handle, [pt.x as f64, pt.y as f64, pt.z as f64]), - ) + .on_object_pick(handle, [pt.x as f64, pt.y as f64, pt.z as f64]) + }) + .map(plugin_step_to_result) + .unwrap_or(crate::command::CmdResult::Cancel) } } diff --git a/src/app/update.rs b/src/app/update.rs index 260291048..08f82c967 100644 --- a/src/app/update.rs +++ b/src/app/update.rs @@ -414,6 +414,14 @@ impl OpenCADStudio { } } + // Surface any plugin-guard panic messages that piled up since + // the last tick (the host singleton isn't reachable from inside + // the plugin hooks, so they queue and we flush here). (#145) + #[cfg(not(target_arch = "wasm32"))] + for msg in crate::plugin::drain_errors() { + self.command_line.push_error(&msg); + } + Task::none() } diff --git a/src/plugin/external.rs b/src/plugin/external.rs index 3d67f0518..b98a77b98 100644 --- a/src/plugin/external.rs +++ b/src/plugin/external.rs @@ -311,7 +311,10 @@ mod loader { let version: libloading::Symbol u32> = lib .get(b"ocs_plugin_api_version") .map_err(|_| "missing ocs_plugin_api_version symbol".to_string())?; - let v = version(); + // Wrap the version + register calls in a panic guard so a plugin + // that panics during load can't take down the host. (#145) + let v = crate::plugin::guard("version", || version()) + .ok_or_else(|| "ocs_plugin_api_version panicked".to_string())?; if v != ocs_plugin_api::API_VERSION { return Err(format!( "API version {v} != host {}", @@ -324,12 +327,16 @@ mod loader { > = lib .get(b"ocs_plugin_register") .map_err(|_| "missing ocs_plugin_register symbol".to_string())?; - let raw = register(); + let raw = crate::plugin::guard("register", || register()) + .ok_or_else(|| "ocs_plugin_register panicked".to_string())?; if raw.is_null() { return Err("ocs_plugin_register returned null".into()); } let plugin = *Box::from_raw(raw); - let id = plugin.manifest().id.to_string(); + // The manifest read happens once at load; guard it too — a buggy + // manifest() that panics here would otherwise crash startup. (#145) + let id = crate::plugin::guard("manifest", || plugin.manifest().id.to_string()) + .ok_or_else(|| "plugin manifest() panicked".to_string())?; Ok(LoadedPlugin { plugin, _lib: lib, diff --git a/src/plugin/mod.rs b/src/plugin/mod.rs index 2578c8bee..c850649a0 100644 --- a/src/plugin/mod.rs +++ b/src/plugin/mod.rs @@ -8,4 +8,72 @@ pub mod marketplace; pub mod registry; pub use registry::{all_ribbon_modules, ribbon_modules_enabled}; -pub(crate) use registry::try_dispatch; \ No newline at end of file +pub(crate) use registry::try_dispatch; + +/// Run a plugin entry point under a panic guard so a buggy external plugin +/// can't take down the host. Returns `None` (after logging) when the plugin +/// panicked; the caller substitutes a safe default. Catches Rust panics across +/// the cdylib boundary — it does NOT contain genuine UB (a plugin that +/// segfaults or violates memory safety via `unsafe` can still crash the host; +/// only out-of-process isolation would). (#145) +pub(crate) fn guard(what: &str, f: impl FnOnce() -> T) -> Option { + match std::panic::catch_unwind(std::panic::AssertUnwindSafe(f)) { + Ok(v) => Some(v), + Err(payload) => { + let msg = payload + .downcast_ref::<&str>() + .map(|s| (*s).to_string()) + .or_else(|| payload.downcast_ref::().cloned()) + .unwrap_or_else(|| "panic".to_string()); + let line = format!("Plugin {what} panicked: {msg} — call ignored"); + eprintln!("[plugin] {line}"); + // The plugin hooks run deep inside the update / render loop where + // the app's command line isn't reachable, so queue the error and + // let the next update tick drain it into the command line. (#145) + errors().lock().unwrap().push(line); + None + } + } +} + +/// Process-wide queue of plugin-guard errors waiting to surface in the host's +/// command-line history. Flushed by [`drain_errors`] from the app loop. +fn errors() -> &'static std::sync::Mutex> { + static E: std::sync::OnceLock>> = std::sync::OnceLock::new(); + E.get_or_init(|| std::sync::Mutex::new(Vec::new())) +} + +/// Take and return every queued plugin-guard error since the last drain. +pub(crate) fn drain_errors() -> Vec { + std::mem::take(&mut *errors().lock().unwrap()) +} + +#[cfg(test)] +mod tests { + use super::guard; + + #[test] + fn returns_value_on_success() { + let r = guard("ok", || 42); + assert_eq!(r, Some(42)); + } + + #[test] + fn swallows_panic_and_queues_error_message() { + let _ = super::drain_errors(); // clear anything from earlier tests + // Silence the default panic-hook stderr noise during the test (the + // guard's own eprintln stays — that's the diagnostic we want). + let prev = std::panic::take_hook(); + std::panic::set_hook(Box::new(|_| {})); + let r: Option = guard("boom", || panic!("plugin went wrong")); + std::panic::set_hook(prev); + assert_eq!(r, None); + // The user-visible message lands in the plugin error queue (the app + // loop drains it into the command line on the next tick). + let queued = super::drain_errors(); + assert!( + queued.iter().any(|m| m.contains("boom") && m.contains("plugin went wrong")), + "expected an error queued for the command line, got: {queued:?}" + ); + } +} \ No newline at end of file diff --git a/src/plugin/registry.rs b/src/plugin/registry.rs index e458b6a75..2fd02678a 100644 --- a/src/plugin/registry.rs +++ b/src/plugin/registry.rs @@ -23,8 +23,15 @@ pub fn ribbon_modules_enabled( let mut addons: Vec<(i32, Box)> = Vec::new(); crate::plugin::external::with_loaded(|loaded| { for lp in loaded { - if !disabled.contains(lp.id.as_str()) { - addons.push((lp.plugin().manifest().ribbon_order, lp.plugin().ribbon())); + if disabled.contains(lp.id.as_str()) { + continue; + } + // Guard the plugin's ribbon build so a panic there can't take + // down the host — the plugin just contributes no tab. (#145) + if let Some(entry) = crate::plugin::guard("ribbon", || { + (lp.plugin().manifest().ribbon_order, lp.plugin().ribbon()) + }) { + addons.push(entry); } } }); @@ -48,7 +55,11 @@ pub(crate) fn try_dispatch(app: &mut OpenCADStudio, tab: usize, cmd: &str) -> bo if disabled.contains(lp.id.as_str()) { continue; } - if lp.plugin().dispatch(&mut host, cmd) { + // A panic inside the plugin's dispatch must not crash the host; + // treat a panicking plugin as "didn't handle it". (#145) + if crate::plugin::guard("dispatch", || lp.plugin().dispatch(&mut host, cmd)) + .unwrap_or(false) + { return true; } }