From a93d4250020b6c05ed7f834e3e02af5f10e49fc1 Mon Sep 17 00:00:00 2001 From: Sebastian <106036+schoeller@users.noreply.github.com> Date: Wed, 24 Jun 2026 12:10:20 +0200 Subject: [PATCH] Fix 3: optimize shutdown times --- crates/ocs_plugin_api/src/process.rs | 108 +++++++++++++++---- crates/ocs_plugin_api/src/process/manager.rs | 20 +++- src/app/mod.rs | 10 ++ src/plugin/external.rs | 11 +- 4 files changed, 128 insertions(+), 21 deletions(-) diff --git a/crates/ocs_plugin_api/src/process.rs b/crates/ocs_plugin_api/src/process.rs index 5424fa05..e7d8399a 100644 --- a/crates/ocs_plugin_api/src/process.rs +++ b/crates/ocs_plugin_api/src/process.rs @@ -1,7 +1,7 @@ //! Process management for out-of-process plugins. use std::path::{Path, PathBuf}; -use std::process::{Child, Command}; +use std::process::{Child, Command, Stdio}; use std::sync::atomic::{AtomicU64, Ordering}; use std::sync::mpsc; use std::sync::Mutex; @@ -82,6 +82,9 @@ impl PluginProcess { .arg("--ocs-plugin-runner") .arg(&socket_name) .arg(cdylib_path) + .stdin(Stdio::null()) + .stdout(Stdio::null()) + .stderr(Stdio::null()) .spawn()?; // Accept the runner connection with a timeout so a hung/crashed runner @@ -237,17 +240,25 @@ impl PluginProcess { } /// Tear down the plugin process without blocking the caller. The stream is - /// closed and the child is killed and reaped in a detached background thread. + /// closed and the child is killed synchronously; the blocking `wait()` is + /// done in a detached background thread so the host never waits on a plugin. pub fn shutdown(&self) { + let (stream, child) = self.take_resources(); + drop(stream); + if let Some(mut child) = child { + let _ = child.kill(); + std::thread::spawn(move || { + let _ = child.wait(); + }); + } + } + + /// Take the stream and child handles out of the process. After this the + /// process is considered shut down and any further IPC will fail. + fn take_resources(&self) -> (Option, Option) { let stream = self.stream.lock().unwrap_or_else(|e| e.into_inner()).take(); let child = self.child.lock().unwrap_or_else(|e| e.into_inner()).take(); - std::thread::spawn(move || { - drop(stream); - if let Some(mut child) = child { - let _ = child.kill(); - let _ = child.wait(); - } - }); + (stream, child) } } @@ -328,21 +339,61 @@ fn call( /// For testing or unusual deployment layouts, set `OCS_PLUGIN_RUNNER_EXE` to /// the host executable path. fn runner_executable() -> Result { - if let Ok(path) = std::env::var("OCS_PLUGIN_RUNNER_EXE") { + static RUNNER: Mutex> = Mutex::new(None); + let mut cached = RUNNER.lock().unwrap_or_else(|e| e.into_inner()); + if let Some(ref path) = *cached { + return Ok(path.clone()); + } + + let path = if let Ok(path) = std::env::var("OCS_PLUGIN_RUNNER_EXE") { let path = PathBuf::from(path); if path.exists() { - return Ok(path); + path + } else { + return Err(PluginError::Runner(format!( + "OCS_PLUGIN_RUNNER_EXE does not exist: {}", + path.display() + ))); } - } - let path = std::env::current_exe()?; - if path.exists() { - Ok(path) } else { - Err(PluginError::Runner(format!( - "cannot find current executable at {}", - path.display() - ))) + let host = std::env::current_exe()?; + if !host.exists() { + return Err(PluginError::Runner(format!( + "cannot find current executable at {}", + host.display() + ))); + } + + // Create a hard link with a distinct name next to the host binary. This + // makes runner processes visible as separate sub-processes in task + // managers / ps, while keeping the runner the exact same binary as the + // host so they can never drift out of sync. + let runner = distinct_runner_path(&host); + let _ = std::fs::remove_file(&runner); + match std::fs::hard_link(&host, &runner) { + Ok(()) => runner, + Err(_) => host, + } + }; + + *cached = Some(path.clone()); + Ok(path) +} + +/// Build a runner path like `-plugin-runner` in the same directory. +/// Using a distinct image name lets task managers show plugin processes as +/// children/sub-processes of the host instead of collapsing them into one row. +fn distinct_runner_path(host: &Path) -> PathBuf { + let mut runner = host.as_os_str().to_owned(); + if let Some(ext) = host.extension().and_then(|s| s.to_str()) { + let base = host.file_stem().unwrap_or_default(); + runner = std::ffi::OsString::from(format!("{}-plugin-runner.{}", base.to_string_lossy(), ext)); + } else { + runner.push("-plugin-runner"); } + let mut path = host.parent().unwrap_or_else(|| Path::new(".")).to_path_buf(); + path.push(runner); + path } /// Generate a unique local socket name. @@ -351,3 +402,22 @@ fn generate_socket_name() -> String { let n = COUNTER.fetch_add(1, Ordering::Relaxed); format!("ocs_plugin_{}_{}", std::process::id(), n) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn distinct_runner_path_appends_suffix() { + let host = PathBuf::from("/app/OpenCADStudio.exe"); + let runner = distinct_runner_path(&host); + assert_eq!(runner, PathBuf::from("/app/OpenCADStudio-plugin-runner.exe")); + } + + #[test] + fn distinct_runner_path_handles_no_extension() { + let host = PathBuf::from("/app/OpenCADStudio"); + let runner = distinct_runner_path(&host); + assert_eq!(runner, PathBuf::from("/app/OpenCADStudio-plugin-runner")); + } +} diff --git a/crates/ocs_plugin_api/src/process/manager.rs b/crates/ocs_plugin_api/src/process/manager.rs index 06ca1bf7..f69b8c80 100644 --- a/crates/ocs_plugin_api/src/process/manager.rs +++ b/crates/ocs_plugin_api/src/process/manager.rs @@ -1,6 +1,7 @@ //! Process manager for out-of-process plugins. use std::path::Path; +use std::process::Child; use std::sync::Arc; use crate::host::HostApi; @@ -120,10 +121,27 @@ impl PluginManager { } /// Begin asynchronous shutdown of every plugin process. + /// + /// Kills every child synchronously on the calling thread and moves the + /// blocking `wait()` calls into a single detached reaper thread, so host + /// shutdown is fast regardless of how many plugins are loaded. pub fn shutdown_all(&mut self) { let plugins = std::mem::take(&mut self.plugins); + let mut children: Vec = Vec::with_capacity(plugins.len()); for p in plugins { - p.process.shutdown(); + let (stream, child) = p.process.take_resources(); + drop(stream); + if let Some(mut child) = child { + let _ = child.kill(); + children.push(child); + } + } + if !children.is_empty() { + std::thread::spawn(move || { + for mut child in children { + let _ = child.wait(); + } + }); } } } diff --git a/src/app/mod.rs b/src/app/mod.rs index 8c6f065e..1989ec24 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -2139,6 +2139,16 @@ pub fn run() -> iced::Result { .run() } +impl Drop for OpenCADStudio { + fn drop(&mut self) { + // Kill plugin runner processes as soon as the application state is + // dropped, instead of waiting for the thread-local manager destructor. + // This makes host shutdown deterministic and fast on every exit path. + #[cfg(not(target_arch = "wasm32"))] + crate::plugin::external::shutdown_plugins(); + } +} + /// Single-window entry for the web (wasm) build. Uses `iced::application` /// instead of `iced::daemon`: the browser canvas is the only window, so the /// main-window view is rendered directly and the manager/dialog windows are diff --git a/src/plugin/external.rs b/src/plugin/external.rs index 360d0fd7..7198beb2 100644 --- a/src/plugin/external.rs +++ b/src/plugin/external.rs @@ -221,7 +221,7 @@ fn parse_string_array(s: &str) -> Vec { // ── Runtime loading (desktop only) ────────────────────────────────────────── #[cfg(not(target_arch = "wasm32"))] -pub(crate) use loader::with_manager; +pub(crate) use loader::{shutdown_plugins, with_manager}; #[cfg(all(not(target_arch = "wasm32"), not(test)))] pub(crate) use loader::{load_at_startup, loaded_ids}; @@ -285,6 +285,15 @@ mod loader { }) } + /// Eagerly shut down all plugin runner processes. + pub fn shutdown_plugins() { + MANAGER.with(|m| { + if let Some(mut manager) = m.borrow_mut().take() { + manager.shutdown_all(); + } + }); + } + /// Path to the native library beside `plugin.toml`, if any. fn lib_file(dir: &Path) -> Option { let ext = lib_extension();