From 83d7a4d7db55d144e0cc40a919e3f1bb575779a8 Mon Sep 17 00:00:00 2001 From: Sebastian <106036+schoeller@users.noreply.github.com> Date: Fri, 14 Aug 2026 20:18:59 +0200 Subject: [PATCH] plugin: parallel release fetches, faster shutdown, uninstall runner stop - Run Plugin Manager release fetches on OS threads so they execute in parallel instead of serialising on the async executor. - Reduce V4Connection shutdown poll from 3s to 100ms; the runner is killed and reaped without blocking host shutdown. - Add PluginManager::remove and PluginProcess::shutdown_and_wait so uninstall can stop the runner before deleting files, fixing Windows access-denied errors on plugin removal. - Rebuild ribbon immediately after uninstall so the tab disappears. --- crates/ocs_plugin_api/src/process.rs | 22 ++++++++++++++++++++ crates/ocs_plugin_api/src/process/manager.rs | 16 ++++++++++++++ crates/ocs_plugin_api/src/process/v4.rs | 8 ++++--- src/app/update/file.rs | 14 +++++++++++-- src/app/update/mod.rs | 7 ++++++- src/plugin/external.rs | 15 ++++++++++++- 6 files changed, 75 insertions(+), 7 deletions(-) diff --git a/crates/ocs_plugin_api/src/process.rs b/crates/ocs_plugin_api/src/process.rs index ecc452e4f..15061318b 100644 --- a/crates/ocs_plugin_api/src/process.rs +++ b/crates/ocs_plugin_api/src/process.rs @@ -722,6 +722,28 @@ impl PluginProcess { } } + /// Tear down the plugin process and block until the child exits or the + /// timeout elapses. Use this when the caller needs the DLL/files to be + /// released before continuing (e.g. uninstalling a plugin on Windows). + pub fn shutdown_and_wait(&self, timeout: Duration) -> Option { + if let Some(v4) = &self.v4 { + v4.shutdown(); + } + let (stream, child) = self.take_resources(); + drop(stream); + if let Some(mut child) = child { + let _ = child.kill(); + let start = Instant::now(); + while start.elapsed() < timeout { + if let Ok(Some(status)) = child.try_wait() { + return Some(status); + } + std::thread::sleep(Duration::from_millis(50)); + } + } + None + } + /// 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) { diff --git a/crates/ocs_plugin_api/src/process/manager.rs b/crates/ocs_plugin_api/src/process/manager.rs index 319be98c3..79524d6a5 100644 --- a/crates/ocs_plugin_api/src/process/manager.rs +++ b/crates/ocs_plugin_api/src/process/manager.rs @@ -2,6 +2,7 @@ use std::path::Path; use std::sync::Arc; +use std::time::Duration; use crate::host::{HostApi, HostNotification, PluginNotification}; use crate::process::{PluginError, PluginProcess}; @@ -206,6 +207,21 @@ impl PluginManager { out } + /// Shut down and remove the plugin with `id` from the manager. + /// + /// Returns `true` if a plugin with that id was found and removed. The + /// process is killed and waited on so its DLL is released; this is intended + /// for uninstall, where the caller needs the files to become deletable on + /// Windows. + pub fn remove(&mut self, id: &str) -> bool { + let Some(index) = self.plugins.iter().position(|p| p.process.id() == id) else { + return false; + }; + let plugin = self.plugins.remove(index); + plugin.process.shutdown_and_wait(Duration::from_secs(5)); + true + } + /// Begin asynchronous shutdown of every plugin process. /// /// Kills every child synchronously on the calling thread and moves the diff --git a/crates/ocs_plugin_api/src/process/v4.rs b/crates/ocs_plugin_api/src/process/v4.rs index a9cfabd06..c079ac396 100644 --- a/crates/ocs_plugin_api/src/process/v4.rs +++ b/crates/ocs_plugin_api/src/process/v4.rs @@ -302,10 +302,12 @@ impl V4Connection { } // Dropping the writer closes the socket. The reader thread will then // exit and clear `alive`. Joining can deadlock if a plugin child - // inherited the socket handle, so we only poll briefly. - let deadline = Instant::now() + Duration::from_secs(3); + // inherited the socket handle, so we only poll briefly. The host process + // will kill the runner afterwards, so a long wait only delays shutdown + // without improving cleanup. + let deadline = Instant::now() + Duration::from_millis(100); while self.shared.alive.load(Ordering::SeqCst) && Instant::now() < deadline { - std::thread::sleep(Duration::from_millis(50)); + std::thread::sleep(Duration::from_millis(10)); } let _ = self.reader_handle.lock().unwrap_or_else(|e| e.into_inner()).take(); } diff --git a/src/app/update/file.rs b/src/app/update/file.rs index 7f89dc2cd..5ba8b14fb 100644 --- a/src/app/update/file.rs +++ b/src/app/update/file.rs @@ -477,12 +477,22 @@ impl OpenCADStudio { } /// Background task: fetch `owner/repo`'s installable releases and their - /// manifest API versions. + /// manifest API versions. The fetch runs on its own OS thread because the + /// several sequential HTTP requests inside `fetch_release_info` would + /// otherwise block the async executor and serialise all repo fetches. #[cfg(not(target_arch = "wasm32"))] pub(in crate::app) fn fetch_releases_task(&self, repo: String) -> Task { let label = repo.clone(); Task::perform( - async move { crate::plugin::marketplace::fetch_release_info(&repo) }, + async move { + let (tx, rx) = iced::futures::channel::oneshot::channel(); + std::thread::spawn(move || { + let result = crate::plugin::marketplace::fetch_release_info(&repo); + let _ = tx.send(result); + }); + rx.await + .unwrap_or_else(|_| Err("release fetch thread died".into())) + }, move |res| Message::PluginReleasesFetched(label, res), ) } diff --git a/src/app/update/mod.rs b/src/app/update/mod.rs index bc43fd08d..4e7f0bbe6 100644 --- a/src/app/update/mod.rs +++ b/src/app/update/mod.rs @@ -5990,11 +5990,16 @@ impl OpenCADStudio { Message::PluginUninstall(id) => { #[cfg(not(target_arch = "wasm32"))] { + // Stop the plugin runner first so Windows releases the DLL + // and allows the package directory to be deleted. + crate::plugin::external::remove_plugin(&id); match crate::plugin::external::uninstall(&id) { Ok(()) => { self.marketplace_status = - format!("Uninstalled '{id}'. Restart to unload it."); + format!("Uninstalled '{id}'."); self.plugin_load_errors.remove(&id); + self.loaded_plugin_ids.remove(&id); + self.rebuild_ribbon_modules(); self.external_plugins = crate::plugin::external::discover(); } Err(e) => { diff --git a/src/plugin/external.rs b/src/plugin/external.rs index c855e7e04..a30ff5264 100644 --- a/src/plugin/external.rs +++ b/src/plugin/external.rs @@ -267,7 +267,7 @@ fn parse_string_array(s: &str) -> Vec { 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}; +pub(crate) use loader::{load_at_startup, loaded_ids, remove_plugin}; #[cfg(not(target_arch = "wasm32"))] #[cfg_attr(test, allow(dead_code))] @@ -342,6 +342,19 @@ mod loader { }); } + /// Shut down the loaded plugin with `id` and remove it from the manager so + /// its files can be deleted on Windows. Returns true if the plugin was + /// loaded and has been removed. + pub fn remove_plugin(id: &str) -> bool { + MANAGER.with(|m| { + if let Some(manager) = m.borrow_mut().as_mut() { + manager.remove(id) + } else { + false + } + }) + } + /// Path to the native library beside `plugin.toml`, if any. fn lib_file(dir: &Path) -> Option { let ext = lib_extension();