From 8df3fa66647a2f8d964d8970b2389cb34ee2a4e9 Mon Sep 17 00:00:00 2001 From: Shunsuke Shibayama Date: Mon, 17 Feb 2025 21:41:33 +0900 Subject: [PATCH 1/2] fix: kill process when the client/server is dead --- rustowl/Cargo.lock | 34 +++++++++++++++++++ rustowl/Cargo.toml | 4 +++ rustowl/src/owlsp.rs | 79 +++++++++++++++++++++++++++++++++++--------- 3 files changed, 101 insertions(+), 16 deletions(-) diff --git a/rustowl/Cargo.lock b/rustowl/Cargo.lock index ee1abbb..86c37cc 100644 --- a/rustowl/Cargo.lock +++ b/rustowl/Cargo.lock @@ -572,6 +572,16 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "process_alive" +version = "0.1.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "fc1b5e484e0aa9dce62b74ddce9fd0cf94b2caeac8fc53a4433a5e8202fa6c47" +dependencies = [ + "libc", + "winapi", +] + [[package]] name = "quote" version = "1.0.38" @@ -600,9 +610,11 @@ checksum = "719b953e2095829ee67db738b3bfa9fa368c94900df327b3f07fe6e794d2fe1f" name = "rustowl" version = "0.1.1" dependencies = [ + "libc", "log", "mktemp", "models", + "process_alive", "rustowl-core", "serde", "serde_json", @@ -960,6 +972,28 @@ version = "0.11.0+wasi-snapshot-preview1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "9c8d87e72b64a3b4db28d11ce29237c246188f4f51057d65a7eab63b7987e423" +[[package]] +name = "winapi" +version = "0.3.9" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "5c839a674fcd7a98952e593242ea400abe93992746761e38641405d28b00f419" +dependencies = [ + "winapi-i686-pc-windows-gnu", + "winapi-x86_64-pc-windows-gnu", +] + +[[package]] +name = "winapi-i686-pc-windows-gnu" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "ac3b87c63620426dd9b991e5ce0329eff545bccbbb34f3be09ff6fb6ab51b7b6" + +[[package]] +name = "winapi-x86_64-pc-windows-gnu" +version = "0.4.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "712e227841d057c1ee1cd2fb22fa7e5a5461ae8e48fa2ca79ec42cfc1931183f" + [[package]] name = "windows-sys" version = "0.48.0" diff --git a/rustowl/Cargo.toml b/rustowl/Cargo.toml index 37ba3ab..9651cd1 100644 --- a/rustowl/Cargo.toml +++ b/rustowl/Cargo.toml @@ -22,6 +22,10 @@ simple_logger = { version = "5.0.0", features = ["stderr"] } tokio.workspace = true tower-lsp = "0.20.0" mktemp = "0.5.1" +process_alive = "0.1.1" + +[target.'cfg(unix)'.dependencies] +libc = "0.2.169" [[bin]] name = "cargo-owl" diff --git a/rustowl/src/owlsp.rs b/rustowl/src/owlsp.rs index c668a57..9fd7549 100644 --- a/rustowl/src/owlsp.rs +++ b/rustowl/src/owlsp.rs @@ -588,6 +588,8 @@ fn search_cargo(p: &PathBuf) -> Vec { res } +type Subprocess = Option; + #[derive(Debug)] struct Backend { #[allow(unused)] @@ -595,6 +597,7 @@ struct Backend { roots: Arc>>, analyzed: Arc>>, processes: Arc>>, + subprocesses: Arc>>, } impl Backend { @@ -604,6 +607,7 @@ impl Backend { roots: Arc::new(RwLock::new(HashMap::new())), analyzed: Arc::new(RwLock::new(None)), processes: Arc::new(RwLock::new(JoinSet::new())), + subprocesses: Arc::new(RwLock::new(vec![])), } } async fn set_roots(&self, uri: &lsp_types::Url) { @@ -623,7 +627,18 @@ impl Backend { } } - async fn analzye(&self) { + async fn abort_subprocess(&self) { + while let Some(pid) = self.subprocesses.write().await.pop() { + if let Some(pid) = pid { + #[cfg(unix)] + unsafe { + libc::killpg(pid.try_into().unwrap(), libc::SIGTERM); + } + } + } + } + + async fn analyze(&self) { // wait for rust-analyzer tokio::time::sleep(tokio::time::Duration::from_millis(100)).await; { @@ -632,21 +647,30 @@ impl Backend { let roots = { self.roots.read().await.clone() }; let mut join = self.processes.write().await; join.shutdown().await; + self.abort_subprocess().await; for (root, target) in roots { - let mut child = process::Command::new("rustup") - .arg("run") - .arg(TOOLCHAIN_VERSION) - .arg("cargo") - .arg("owl") - .arg(&root) - .arg(&target) - .stdout(std::process::Stdio::piped()) - .stderr(std::process::Stdio::null()) - .spawn() - .unwrap(); + let mut child = unsafe { + process::Command::new("rustup") + .arg("run") + .arg(TOOLCHAIN_VERSION) + .arg("cargo") + .arg("owl") + .arg(&root) + .arg(&target) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::null()) + .kill_on_drop(true) + .pre_exec(|| { + #[cfg(unix)] + libc::setsid(); + Ok(()) + }) + .spawn() + .unwrap() + }; let mut stdout = BufReader::new(child.stdout.take().unwrap()).lines(); let analyzed = self.analyzed.clone(); - tokio::spawn(async move { + join.spawn(async move { while let Ok(Some(line)) = stdout.next_line().await { if let Ok(ws) = serde_json::from_str::(&line) { let write = &mut *analyzed.write().await; @@ -658,9 +682,11 @@ impl Backend { } } }); + let pid = child.id(); join.spawn(async move { let _ = child.wait().await; }); + self.subprocesses.write().await.push(pid); } } async fn cleanup_targets(&self) { @@ -721,6 +747,15 @@ impl Backend { } } +impl Drop for Backend { + fn drop(&mut self) { + let rt = tokio::runtime::Runtime::new().unwrap(); + rt.block_on(async { + self.shutdown().await.unwrap(); + }); + } +} + #[tower_lsp::async_trait] impl LanguageServer for Backend { async fn initialize( @@ -753,13 +788,24 @@ impl LanguageServer for Backend { capabilities: server_cap, ..Default::default() }; + let health_checker = async move { + if let Some(process_id) = params.process_id { + loop { + tokio::time::sleep(tokio::time::Duration::from_secs(30)).await; + if !process_alive::state(process_alive::Pid::from(process_id)).is_alive() { + panic!("The client process is dead"); + } + } + } + }; + tokio::spawn(health_checker); Ok(init_res) } async fn initialized(&self, _p: lsp_types::InitializedParams) { - self.analzye().await; + self.analyze().await; } async fn did_save(&self, _params: lsp_types::DidSaveTextDocumentParams) { - self.analzye().await; + self.analyze().await; } async fn did_change(&self, _params: lsp_types::DidChangeTextDocumentParams) { *self.analyzed.write().await = None; @@ -773,12 +819,13 @@ impl LanguageServer for Backend { for added in params.event.added { self.set_roots(&added.uri).await; } - self.analzye().await; + self.analyze().await; } async fn shutdown(&self) -> jsonrpc::Result<()> { self.cleanup_targets().await; self.processes.write().await.shutdown().await; + self.abort_subprocess().await; Ok(()) } } From f96560dd8cee2fed3a081b952269dd392f71d02b Mon Sep 17 00:00:00 2001 From: Shunsuke Shibayama Date: Tue, 18 Feb 2025 20:43:25 +0900 Subject: [PATCH 2/2] improve error handling --- rustowl/src/owlsp.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/rustowl/src/owlsp.rs b/rustowl/src/owlsp.rs index 9fd7549..bfa3ea4 100644 --- a/rustowl/src/owlsp.rs +++ b/rustowl/src/owlsp.rs @@ -749,9 +749,17 @@ impl Backend { impl Drop for Backend { fn drop(&mut self) { - let rt = tokio::runtime::Runtime::new().unwrap(); + let rt = match tokio::runtime::Runtime::new() { + Ok(rt) => rt, + Err(err) => { + log::error!("failed to create async runtime for graceful shutdown: {err}"); + return; + } + }; rt.block_on(async { - self.shutdown().await.unwrap(); + if let Err(err) = self.shutdown().await { + log::error!("failed to shutdown the server gracefully: {err}"); + }; }); } }