diff --git a/crates/ide-host/src/lib.rs b/crates/ide-host/src/lib.rs index b0eded26..4d8e9cd2 100644 --- a/crates/ide-host/src/lib.rs +++ b/crates/ide-host/src/lib.rs @@ -82,8 +82,13 @@ pub struct Host { pub(crate) external_logs: RwLock>>>, requests: requests::Requests, project_requests: RwLock>>, + /// Shutdowns of projects closed with [`Host::close_project_detached`]. + closing: parking_lot::Mutex>>, } +/// How long exit waits for project shutdown before giving up. +const EXIT_SHUTDOWN_WAIT: std::time::Duration = std::time::Duration::from_secs(10); + impl Host { pub fn new(config: HostConfig) -> Self { let HostConfig { @@ -126,6 +131,7 @@ impl Host { external_logs: RwLock::default(), requests: Default::default(), project_requests: RwLock::default(), + closing: Default::default(), }; host.apply_proxy(); host @@ -179,46 +185,69 @@ impl Host { } } + /// Closes `window`'s project and waits until its language servers, kernels and + /// debug sessions have shut down. Never call this on the UI event-loop thread: + /// the shutdown runs on the async runtime, whose workers may themselves be + /// waiting for the event loop (IPC responses, window queries). Use + /// [`Self::close_project_detached`] there. pub fn close_project(&self, window: &str) { + if let Some(shutdown) = self.begin_close(window) { + self.wait_blocking(shutdown); + } + } + + /// Like [`Self::close_project`], but returns once the project is detached from + /// the window; its workers finish shutting down in the background, and + /// [`Self::shutdown_notebooks_on_exit`] waits for them before the process exits. + pub fn close_project_detached(&self, window: &str) { + if let Some(shutdown) = self.begin_close(window) { + let mut closing = self.closing.lock(); + closing.retain(|handle| !handle.is_finished()); + closing.push(shutdown); + } + } + + /// Detaches the project from `window` and stops what it started, synchronously, + /// so the window can open another project at once. Returns the thread that + /// finishes shutting down its workers. + fn begin_close(&self, window: &str) -> Option> { self.external_logs.write().remove(self.owner_window(window)); for request in self.project_requests.write().remove(self.owner_window(window)).unwrap_or_default() { self.cancel_request(request); } - let project = self.projects.write().remove(window); - if let Some(project) = project { - if !project.light { - self.environment.project_closed(); - } - if let Some(bridge) = &project.kotlin_bridge { - bridge.stop(); - } - project.documents.clear_workspace_edit_history(); - project.agents.shutdown(); - project.notebooks.stop_admission(); - // What the closing window started goes with it — the builds and searches it - // asked for, its runs, its shells and its debug sessions — and the language - // servers finish shutting down before the window is gone, as IDEA closes a - // project's processes. - self.processes.kill_project(&project.root); - self.terminals.kill_key(self.owner_window(window)); - self.shutdown_project_workers_blocking( - vec![project.notebooks.clone()], - project.kotlin_bridge.iter().cloned().collect(), - Some(project.lsp.clone()), - Some((self.debugger.clone(), window.to_owned())), - ); + let project = self.projects.write().remove(window)?; + if !project.light { + self.environment.project_closed(); } + if let Some(bridge) = &project.kotlin_bridge { + bridge.stop(); + } + project.documents.clear_workspace_edit_history(); + project.agents.shutdown(); + project.notebooks.stop_admission(); + // What the closing window started goes with it — the builds and searches it + // asked for, its runs, its shells and its debug sessions — and its language + // servers shut down. + self.processes.kill_project(&project.root); + self.terminals.kill_key(self.owner_window(window)); + Some(self.spawn_project_shutdown( + vec![project.notebooks.clone()], + project.kotlin_bridge.iter().cloned().collect(), + Some(project.lsp.clone()), + Some((self.debugger.clone(), window.to_owned())), + )) } - /// Synchronous project/window destruction must finish owned kernel cleanup, - /// not enqueue work which can be lost as the desktop runtime exits. - fn shutdown_project_workers_blocking( + /// Project/window destruction must finish owned kernel cleanup, not enqueue + /// work which can be lost as the desktop runtime exits: the returned thread is + /// joined by the closer or, for a detached close, before exit. + fn spawn_project_shutdown( &self, notebooks: Vec>, bridges: Vec>, lsp: Option>, debug: Option<(Arc, String)>, - ) { + ) -> std::thread::JoinHandle<()> { for bridge in &bridges { bridge.stop(); } @@ -226,25 +255,37 @@ impl Host { service.stop_admission(); } let runtime = self.runtime.clone(); - let wait = move || { - let worker = std::thread::spawn(move || { - runtime.block_on(async move { - for bridge in bridges { - bridge.shutdown().await; - } - for service in notebooks { - service.shutdown().await; - } - if let Some(lsp) = lsp { - lsp.shutdown_all().await; - } - if let Some((debug, key)) = debug { - debug.stop_key(&key).await; - } - }); + std::thread::spawn(move || { + runtime.block_on(async move { + for bridge in bridges { + bridge.shutdown().await; + } + for service in notebooks { + service.shutdown().await; + } + if let Some(lsp) = lsp { + lsp.shutdown_all().await; + } + if let Some((debug, key)) = debug { + debug.stop_key(&key).await; + } }); - if let Err(error) = worker.join() { - tracing::error!("notebook shutdown thread failed: {error:?}"); + }) + } + + /// Waits for detached closes, so a project opened next never races the + /// language servers of one still shutting down (e.g. over a shared workspace). + pub(crate) fn wait_detached_closes(&self) { + let pending: Vec<_> = self.closing.lock().drain(..).collect(); + for shutdown in pending { + self.wait_blocking(shutdown); + } + } + + fn wait_blocking(&self, shutdown: std::thread::JoinHandle<()>) { + let wait = move || { + if let Err(error) = shutdown.join() { + tracing::error!("project shutdown thread failed: {error:?}"); } }; if tokio::runtime::Handle::try_current() @@ -256,15 +297,32 @@ impl Host { } } - /// Called before the desktop runtime exits, including update/restart exits. + /// Called before the desktop runtime exits, including update/restart exits, on + /// the event-loop thread: waits for detached closes and the open projects' + /// kernels, but no longer than [`EXIT_SHUTDOWN_WAIT`], so a runtime worker + /// stuck on the event loop cannot keep the process from exiting. pub fn shutdown_notebooks_on_exit(&self) { let projects = self.projects(); - self.shutdown_project_workers_blocking( + let mut pending: Vec<_> = self.closing.lock().drain(..).collect(); + pending.push(self.spawn_project_shutdown( projects.iter().map(|project| project.notebooks.clone()).collect(), projects.iter().filter_map(|project| project.kotlin_bridge.clone()).collect(), None, None, - ); + )); + let deadline = std::time::Instant::now() + EXIT_SHUTDOWN_WAIT; + while pending.iter().any(|handle| !handle.is_finished()) { + if std::time::Instant::now() >= deadline { + tracing::warn!("project shutdown did not finish within {EXIT_SHUTDOWN_WAIT:?}; exiting anyway"); + return; + } + std::thread::sleep(std::time::Duration::from_millis(20)); + } + for handle in pending { + if let Err(error) = handle.join() { + tracing::error!("project shutdown thread failed: {error:?}"); + } + } } /// Called before desktop exit, which may bypass document destructors. diff --git a/crates/ide-host/src/project.rs b/crates/ide-host/src/project.rs index 1213db96..11df9189 100644 --- a/crates/ide-host/src/project.rs +++ b/crates/ide-host/src/project.rs @@ -88,6 +88,7 @@ impl Host { return Ok(current); } self.close_project(window); + self.wait_detached_closes(); let sink: SharedSink = (self.sink_for)(window); // Before anything the project starts: its children get the project's environment. let trusted = Arc::new(AtomicBool::new(self.trust.trusted(&root)?)); @@ -567,6 +568,7 @@ impl Host { return Ok(current); } self.close_project(window); + self.wait_detached_closes(); let sink: SharedSink = (self.sink_for)(window); let settings = Arc::new(SettingsStore::load(&self.config_dir, None)); let effective = settings.effective(); diff --git a/crates/ide-host/tests/close_project.rs b/crates/ide-host/tests/close_project.rs index e0b07cd1..3da5acbd 100644 --- a/crates/ide-host/tests/close_project.rs +++ b/crates/ide-host/tests/close_project.rs @@ -5,6 +5,7 @@ use ide_core::NullSink; use ide_host::{Host, HostConfig, ops}; use std::sync::Arc; +use std::time::{Duration, Instant}; fn options(root: &std::path::Path) -> ide_terminal::TerminalOptions { ide_terminal::TerminalOptions { @@ -52,3 +53,53 @@ fn closing_a_project_closes_its_terminals_and_keeps_other_windows() { host.terminals.kill_key("other"); }); } + +/// The desktop closes a project on the event-loop thread while runtime workers may be +/// blocked waiting for that thread (IPC responses, window queries): closing must not +/// wait for workers that need the runtime, and exit still waits for them once free. +#[test] +fn detached_close_returns_while_runtime_workers_wait_for_the_closing_thread() { + let tmp = tempfile::tempdir().unwrap(); + let root = dunce::canonicalize(tmp.path()).unwrap().join("project"); + std::fs::create_dir_all(&root).unwrap(); + let runtime = tokio::runtime::Builder::new_multi_thread().worker_threads(1).enable_all().build().unwrap(); + let _guard = runtime.enter(); + let host = Arc::new(Host::new(HostConfig { + config_dir: tmp.path().join("config"), + data_dir: tmp.path().join("data"), + local_data_dir: tmp.path().join("data"), + languages: Arc::new(ide_language::LanguageRegistry::load(None).unwrap()), + syntax: Arc::default(), + sink_for: Box::new(|_| Arc::new(NullSink)), + owner: |w| w, + runtime: runtime.handle().clone(), + project_environment: false, + })); + host.trust.decide(&root, true, false).unwrap(); + host.open_project("main", &root).unwrap(); + + // Occupy the only runtime worker until "the event loop" answers. + let (started_tx, started_rx) = std::sync::mpsc::channel(); + let (release_tx, release_rx) = std::sync::mpsc::channel::<()>(); + runtime.spawn(async move { + started_tx.send(()).unwrap(); + let _ = release_rx.recv(); + }); + started_rx.recv().unwrap(); + + let (closed_tx, closed_rx) = std::sync::mpsc::channel(); + let closer = host.clone(); + std::thread::spawn(move || { + closer.close_project_detached("main"); + let _ = closed_tx.send(()); + }); + let returned = closed_rx.recv_timeout(Duration::from_secs(5)); + release_tx.send(()).unwrap(); + assert!(returned.is_ok(), "closing waited for a runtime worker that waits for the closing thread"); + assert!(host.project("main").is_err(), "the window no longer has the project"); + host.open_project("main", &root).expect("the window opens a project once the close has finished"); + + let started = Instant::now(); + host.shutdown_notebooks_on_exit(); + assert!(started.elapsed() < Duration::from_secs(5), "exit joined the finished close instead of timing out"); +} diff --git a/src-tauri/src/state.rs b/src-tauri/src/state.rs index 34011eb7..f651c71f 100644 --- a/src-tauri/src/state.rs +++ b/src-tauri/src/state.rs @@ -154,9 +154,11 @@ impl AppState { self.host.open_light(window, file) } + /// Called on the event-loop thread (window destruction, the `close_project` + /// command), so it must not wait for the project's workers to shut down. pub fn close_project(&self, window: &str) { self.remote_dev.disconnect(window); - self.host.close_project(window); + self.host.close_project_detached(window); } }