Fix 2: optimize src

crates/ocs_plugin_api/src/lib.rs

@@ -43,4 +43,4 @@ pub use ribbon::{
 };
 
 #[cfg(feature = "host")]
-pub use process::{PluginError, PluginProcess};
+pub use process::{DispatchResult, PluginError, PluginManager, PluginProcess};

crates/ocs_plugin_api/src/process.rs

@@ -18,6 +18,11 @@ use crate::ipc::server::handle_plugin_request;
 use crate::ipc::transport::{recv, send};
 use crate::ribbon::owned::{OwnedPluginManifest, OwnedRibbonGroup as OwnedRibbonGroupAlias};
 
+use serde::de::DeserializeOwned;
+
+mod manager;
+pub use manager::{DispatchResult, PluginManager};
+
 /// Maximum time to wait for the plugin runner to connect back to the host.
 const SPAWN_TIMEOUT: Duration = Duration::from_secs(10);
 
@@ -47,8 +52,8 @@ pub enum PluginError {
 
 /// One spawned plugin process.
 pub struct PluginProcess {
-    stream: Mutex<Stream>,
-    child: Mutex<Child>,
+    stream: Mutex<Option<Stream>>,
+    child: Mutex<Option<Child>>,
     id: String,
     manifest: OwnedPluginManifest,
     ribbon: Vec<OwnedRibbonGroupAlias>,
@@ -88,7 +93,7 @@ impl PluginProcess {
         let stream = match rx.recv_timeout(spawn_timeout()) {
             Ok(Ok(stream)) => {
                 eprintln!("[plugin] runner connected");
-                Mutex::new(stream)
+                Mutex::new(Some(stream))
             }
             Ok(Err(e)) => return Err(e.into()),
             Err(mpsc::RecvTimeoutError::Timeout) => {
@@ -116,7 +121,7 @@ impl PluginProcess {
         let id = manifest.id.clone();
         Ok(Self {
             stream,
-            child: Mutex::new(child),
+            child: Mutex::new(Some(child)),
             id,
             manifest,
             ribbon,
@@ -165,12 +170,9 @@ impl PluginProcess {
         command_id: u64,
         event: InteractiveEvent,
     ) -> Result<CommandStep, PluginError> {
-        send(
-            &mut self.stream.lock().unwrap(),
-            &HostToPlugin::Request(HostRequest::InteractiveEvent { command_id, event }),
-        )?;
+        self.send_request(HostRequest::InteractiveEvent { command_id, event })?;
         loop {
-            match recv::<PluginToHost>(&mut self.stream.lock().unwrap())? {
+            match self.recv_response::<PluginToHost>()? {
                 PluginToHost::Response(HostResponse::CommandStep(s)) => return Ok(s),
                 PluginToHost::Response(other) => {
                     return Err(PluginError::UnexpectedResponse(other))
@@ -179,7 +181,7 @@ impl PluginProcess {
                     let resp = crate::ipc::protocol::PluginResponse::Error(format!(
                         "unexpected nested request during interactive event: {req:?}"
                     ));
-                    send(&mut self.stream.lock().unwrap(), &HostToPlugin::Response(resp))?;
+                    self.send_response(resp)?;
                 }
             }
         }
@@ -187,12 +189,9 @@ impl PluginProcess {
 
     /// Ask the plugin process for the current prompt of an interactive command.
     pub fn get_prompt(&self, command_id: u64) -> Result<String, PluginError> {
-        send(
-            &mut self.stream.lock().unwrap(),
-            &HostToPlugin::Request(HostRequest::GetPrompt { command_id }),
-        )?;
+        self.send_request(HostRequest::GetPrompt { command_id })?;
         loop {
-            match recv::<PluginToHost>(&mut self.stream.lock().unwrap())? {
+            match self.recv_response::<PluginToHost>()? {
                 PluginToHost::Response(HostResponse::Text(s)) => return Ok(s),
                 PluginToHost::Response(other) => {
                     return Err(PluginError::UnexpectedResponse(other))
@@ -201,7 +200,7 @@ impl PluginProcess {
                     let resp = crate::ipc::protocol::PluginResponse::Error(format!(
                         "unexpected nested request during get_prompt: {req:?}"
                     ));
-                    send(&mut self.stream.lock().unwrap(), &HostToPlugin::Response(resp))?;
+                    self.send_response(resp)?;
                 }
             }
         }
@@ -209,12 +208,9 @@ impl PluginProcess {
 
     /// Ask the plugin process whether an interactive command wants object picks.
     pub fn needs_entity_pick(&self, command_id: u64) -> Result<bool, PluginError> {
-        send(
-            &mut self.stream.lock().unwrap(),
-            &HostToPlugin::Request(HostRequest::NeedsEntityPick { command_id }),
-        )?;
+        self.send_request(HostRequest::NeedsEntityPick { command_id })?;
         loop {
-            match recv::<PluginToHost>(&mut self.stream.lock().unwrap())? {
+            match self.recv_response::<PluginToHost>()? {
                 PluginToHost::Response(HostResponse::Bool(b)) => return Ok(b),
                 PluginToHost::Response(other) => {
                     return Err(PluginError::UnexpectedResponse(other))
@@ -223,63 +219,105 @@ impl PluginProcess {
                     let resp = crate::ipc::protocol::PluginResponse::Error(format!(
                         "unexpected nested request during needs_entity_pick: {req:?}"
                     ));
-                    send(&mut self.stream.lock().unwrap(), &HostToPlugin::Response(resp))?;
+                    self.send_response(resp)?;
                 }
             }
         }
     }
 
     pub fn is_alive(&self) -> bool {
-        match self.child.lock().unwrap().try_wait() {
-            Ok(None) => true,
-            Ok(Some(_)) | Err(_) => false,
+        let mut guard = self.child.lock().unwrap_or_else(|e| e.into_inner());
+        match guard.as_mut() {
+            Some(child) => match child.try_wait() {
+                Ok(None) => true,
+                Ok(Some(_)) | Err(_) => false,
+            },
+            None => false,
         }
     }
 
-    pub fn kill(&self) -> std::io::Result<()> {
-        let _ = call_no_host(&self.stream, HostRequest::Shutdown);
-        self.child.lock().unwrap().kill()
+    /// 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.
+    pub fn shutdown(&self) {
+        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();
+            }
+        });
     }
 }
 
 impl Drop for PluginProcess {
     fn drop(&mut self) {
-        let _ = self.kill();
+        self.shutdown();
     }
 }
 
+impl PluginProcess {
+    fn send_request(&self, req: HostRequest) -> Result<(), PluginError> {
+        let mut guard = self.stream.lock().unwrap_or_else(|e| e.into_inner());
+        let stream = guard.as_mut().ok_or_else(shutdown_error)?;
+        send(stream, &HostToPlugin::Request(req)).map_err(Into::into)
+    }
+
+    fn send_response(&self, resp: crate::ipc::protocol::PluginResponse) -> Result<(), PluginError> {
+        let mut guard = self.stream.lock().unwrap_or_else(|e| e.into_inner());
+        let stream = guard.as_mut().ok_or_else(shutdown_error)?;
+        send(stream, &HostToPlugin::Response(resp)).map_err(Into::into)
+    }
+
+    fn recv_response<T: DeserializeOwned>(&self) -> Result<T, PluginError> {
+        let mut guard = self.stream.lock().unwrap_or_else(|e| e.into_inner());
+        let stream = guard.as_mut().ok_or_else(shutdown_error)?;
+        recv(stream).map_err(Into::into)
+    }
+}
+
+fn shutdown_error() -> PluginError {
+    PluginError::Io(std::io::Error::new(
+        std::io::ErrorKind::NotConnected,
+        "plugin process has been shut down",
+    ))
+}
+
 /// Send a host request and wait for the response, handling any nested plugin
 /// requests inline using the supplied `HostApi`.
 fn call(
-    stream: &Mutex<Stream>,
+    stream: &Mutex<Option<Stream>>,
     host: &mut dyn HostApi,
     req: HostRequest,
     on_start_interactive: &mut dyn FnMut(u64),
 ) -> Result<HostResponse, PluginError> {
     eprintln!("[plugin] host -> runner: {req:?}");
-    send(&mut stream.lock().unwrap(), &HostToPlugin::Request(req))?;
+    {
+        let mut guard = stream.lock().unwrap_or_else(|e| e.into_inner());
+        let stream = guard.as_mut().ok_or_else(shutdown_error)?;
+        send(stream, &HostToPlugin::Request(req))?;
+    }
     loop {
-        let msg = recv::<PluginToHost>(&mut stream.lock().unwrap())?;
+        let msg = {
+            let mut guard = stream.lock().unwrap_or_else(|e| e.into_inner());
+            let stream = guard.as_mut().ok_or_else(shutdown_error)?;
+            recv::<PluginToHost>(stream)?
+        };
         eprintln!("[plugin] runner -> host: {msg:?}");
         match msg {
             PluginToHost::Response(resp) => return Ok(resp),
             PluginToHost::Request(plugin_req) => {
                 let resp = handle_plugin_request(host, plugin_req, on_start_interactive);
                 eprintln!("[plugin] host -> runner response: {resp:?}");
-                send(&mut stream.lock().unwrap(), &HostToPlugin::Response(resp))?;
+                let mut guard = stream.lock().unwrap_or_else(|e| e.into_inner());
+                let stream = guard.as_mut().ok_or_else(shutdown_error)?;
+                send(stream, &HostToPlugin::Response(resp))?;
             }
         }
     }
 }
 
-/// Best-effort shutdown request that does not wait for a response.
-fn call_no_host(
-    stream: &Mutex<Stream>,
-    req: HostRequest,
-) -> Result<(), crate::ipc::transport::TransportError> {
-    send(&mut stream.lock().unwrap(), &HostToPlugin::Request(req))
-}
-
 /// Locate the executable to spawn for running a plugin.
 ///
 /// The host spawns *itself* in runner mode (`--ocs-plugin-runner`), so the

crates/ocs_plugin_api/src/process/manager.rs

@@ -0,0 +1,223 @@
+//! Process manager for out-of-process plugins.
+
+use std::path::Path;
+use std::sync::Arc;
+
+use crate::host::HostApi;
+use crate::process::{PluginError, PluginProcess};
+use crate::ribbon::owned::{to_shared_module, SharedCadModule};
+
+/// Owner of every spawned plugin process.
+pub struct PluginManager {
+    plugins: Vec<LoadedPlugin>,
+}
+
+struct LoadedPlugin {
+    process: Arc<PluginProcess>,
+    module: SharedCadModule,
+}
+
+impl Default for PluginManager {
+    fn default() -> Self {
+        Self::new()
+    }
+}
+
+/// Outcome of [`PluginManager::dispatch`].
+#[derive(Default)]
+pub struct DispatchResult {
+    /// A plugin handled the command.
+    pub handled: bool,
+    /// An interactive command was started by a plugin.
+    pub started: Option<(Arc<PluginProcess>, u64)>,
+    /// Plugins whose process died before or during dispatch.
+    pub dead_plugins: Vec<String>,
+    /// Plugins that returned an error while trying to handle the command.
+    pub errors: Vec<(String, String)>,
+}
+
+impl PluginManager {
+    /// Create an empty manager.
+    pub fn new() -> Self {
+        Self { plugins: Vec::new() }
+    }
+
+    /// Spawn `cdylib_path` as a separate plugin process, build its ribbon
+    /// module, and store it. Returns the plugin id on success.
+    pub fn load(
+        &mut self,
+        cdylib_path: &Path,
+        host: &mut dyn HostApi,
+    ) -> Result<String, PluginError> {
+        let process = PluginProcess::spawn(cdylib_path, host)?;
+        let id = process.id().to_string();
+        let name = process.manifest().name.clone();
+        let module = to_shared_module(id.clone(), name, process.ribbon().to_vec());
+        self.plugins.push(LoadedPlugin {
+            process: Arc::new(process),
+            module,
+        });
+        Ok(id)
+    }
+
+    /// Ribbon modules for alive, non-disabled plugins, sorted by `ribbon_order`.
+    pub fn ribbon_modules<F: Fn(&str) -> bool>(
+        &self,
+        is_disabled: F,
+    ) -> Vec<(i32, SharedCadModule)> {
+        let mut out: Vec<(i32, SharedCadModule)> = self
+            .plugins
+            .iter()
+            .filter(|p| !is_disabled(p.process.id()) && p.process.is_alive())
+            .map(|p| (p.process.manifest().ribbon_order, p.module.clone()))
+            .collect();
+        out.sort_by_key(|(order, _)| *order);
+        out
+    }
+
+    /// Dispatch `cmd` to each plugin until one handles it.
+    ///
+    /// `is_disabled` is called for each plugin id so the host can filter
+    /// disabled plugins without exposing its set type to the crate.
+    pub fn dispatch<F: Fn(&str) -> bool>(
+        &self,
+        host: &mut dyn HostApi,
+        cmd: &str,
+        is_disabled: F,
+    ) -> DispatchResult {
+        let mut result = DispatchResult::default();
+        for p in &self.plugins {
+            let id = p.process.id().to_string();
+            if is_disabled(&id) {
+                continue;
+            }
+            if !p.process.is_alive() {
+                result.dead_plugins.push(id);
+                continue;
+            }
+            let process = Arc::clone(&p.process);
+            let mut on_start = |command_id: u64| {
+                result.started = Some((Arc::clone(&process), command_id));
+            };
+            match p.process.dispatch(host, cmd, &mut on_start) {
+                Ok(true) => {
+                    result.handled = true;
+                    return result;
+                }
+                Ok(false) => {}
+                Err(e) => result.errors.push((id, e.to_string())),
+            }
+        }
+        result
+    }
+
+    /// Plugin ids currently loaded.
+    pub fn ids(&self) -> Vec<String> {
+        self.plugins
+            .iter()
+            .map(|p| p.process.id().to_string())
+            .collect()
+    }
+
+    /// Begin asynchronous shutdown of every plugin process.
+    pub fn shutdown_all(&mut self) {
+        let plugins = std::mem::take(&mut self.plugins);
+        for p in plugins {
+            p.process.shutdown();
+        }
+    }
+}
+
+impl Drop for PluginManager {
+    fn drop(&mut self) {
+        self.shutdown_all();
+    }
+}
+
+#[cfg(all(test, feature = "host"))]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn empty_manager_has_no_ribbon_modules() {
+        let manager = PluginManager::new();
+        assert!(manager.ribbon_modules(|_| false).is_empty());
+        assert!(manager.ids().is_empty());
+    }
+
+    #[test]
+    fn dispatch_with_no_plugins_is_not_handled() {
+        struct DummyHost;
+        impl HostApi for DummyHost {
+            fn tab_index(&self) -> usize {
+                0
+            }
+            fn document(&self) -> &acadrust::CadDocument {
+                panic!("not used")
+            }
+            fn document_mut(&mut self) -> &mut acadrust::CadDocument {
+                panic!("not used")
+            }
+            fn add_entity(&mut self, _entity: acadrust::EntityType) -> acadrust::Handle {
+                panic!("not used")
+            }
+            fn bump_geometry(&mut self) {}
+            fn read_record(
+                &self,
+                _handle: acadrust::Handle,
+                _app_name: &str,
+            ) -> Option<&acadrust::xdata::ExtendedDataRecord> {
+                None
+            }
+            fn write_record(
+                &mut self,
+                _handle: acadrust::Handle,
+                _record: acadrust::xdata::ExtendedDataRecord,
+            ) -> bool {
+                false
+            }
+            fn remove_record(
+                &mut self,
+                _handle: acadrust::Handle,
+                _app_name: &str,
+            ) -> bool {
+                false
+            }
+            fn push_undo(&mut self, _label: &str) {}
+            fn set_dirty(&mut self) {}
+            fn push_info(&mut self, _msg: &str) {}
+            fn push_output(&mut self, _msg: &str) {}
+            fn push_error(&mut self, _msg: &str) {}
+            fn start_interactive(
+                &mut self,
+                _command: Box<dyn crate::host::InteractiveCommand>,
+            ) {
+            }
+            fn plugin_state_any(
+                &self,
+                _plugin_id: &str,
+            ) -> Option<&(dyn std::any::Any + Send + Sync)> {
+                None
+            }
+            fn plugin_state_any_mut(
+                &mut self,
+                _plugin_id: &str,
+            ) -> Option<&mut (dyn std::any::Any + Send + Sync)> {
+                None
+            }
+            fn ensure_plugin_state_any(
+                &mut self,
+                _plugin_id: &'static str,
+                _init: &mut dyn FnMut() -> Box<dyn std::any::Any + Send + Sync>,
+            ) -> &mut (dyn std::any::Any + Send + Sync) {
+                panic!("not used")
+            }
+        }
+
+        let manager = PluginManager::new();
+        let mut host = DummyHost;
+        let result = manager.dispatch(&mut host, "FOO", |_| false);
+        assert!(!result.handled);
+        assert!(result.started.is_none());
+    }
+}