diff --git a/crates/ide-core/src/process.rs b/crates/ide-core/src/process.rs index e40a9d069..0f0b47e1d 100644 --- a/crates/ide-core/src/process.rs +++ b/crates/ide-core/src/process.rs @@ -104,6 +104,9 @@ fn signal_group(child: &Child, signal: libc::c_int) { #[cfg(windows)] const TREE_EXIT_WAIT: Duration = Duration::from_secs(5); +#[cfg(windows)] +pub use windows_tree::ProcessTree; + /// `taskkill /T /F` from `pid`, then waits (up to [`TREE_EXIT_WAIT`]) until what the child /// started is gone: termination is asynchronous, and until a process is gone the files it /// holds open can't be opened or deleted. The child itself is left to its owner to reap. @@ -136,13 +139,48 @@ mod windows_tree { CreateToolhelp32Snapshot, PROCESSENTRY32W, Process32FirstW, Process32NextW, TH32CS_SNAPPROCESS, }; use windows_sys::Win32::System::Threading::{ - GetProcessTimes, OpenProcess, PROCESS_QUERY_LIMITED_INFORMATION, PROCESS_SYNCHRONIZE, WaitForSingleObject, + GetProcessTimes, OpenProcess, PROCESS_QUERY_LIMITED_INFORMATION, PROCESS_SYNCHRONIZE, PROCESS_TERMINATE, + TerminateProcess, WaitForSingleObject, }; + /// Owned handles to a program and its current descendants, retained even if its launcher exits. + pub struct ProcessTree { + processes: Vec, + } + + impl ProcessTree { + pub fn capture(pid: u32) -> Self { + let Some(root) = open_with(pid, PROCESS_TERMINATE) else { return Self { processes: Vec::new() } }; + let children = descendants_with(pid, PROCESS_TERMINATE); + Self { processes: std::iter::once(root).chain(children).collect() } + } + + pub fn is_running(&self) -> bool { + self.processes.iter().any(|process| { + // SAFETY: an owned process handle opened with SYNCHRONIZE access. + unsafe { + WaitForSingleObject(process.as_raw_handle(), 0) == windows_sys::Win32::Foundation::WAIT_TIMEOUT + } + }) + } + + /// Requests termination children first; poll `is_running` until termination completes. + pub fn terminate(&self) { + for process in self.processes.iter().rev() { + // SAFETY: an owned process handle opened with TERMINATE access, never a reused pid. + unsafe { TerminateProcess(process.as_raw_handle(), 1) }; + } + } + } + /// Every process `root` started, directly or not, opened so it can be waited for. A /// parent's id is reused once it is gone, so a process counts only if it started after /// its parent. pub(super) fn descendants(root: u32) -> Vec { + descendants_with(root, 0) + } + + fn descendants_with(root: u32, access: u32) -> Vec { let Some(root_started) = open(root).as_ref().and_then(started) else { return Vec::new() }; let processes = processes(); let mut seen = HashSet::from([root]); @@ -153,7 +191,7 @@ mod windows_tree { if ppid != parent || !seen.insert(pid) { continue; } - let Some(handle) = open(pid) else { continue }; + let Some(handle) = open_with(pid, access) else { continue }; if let Some(at) = started(&handle).filter(|&at| at >= parent_started) { parents.push((pid, at)); found.push(handle); @@ -193,8 +231,12 @@ mod windows_tree { } fn open(pid: u32) -> Option { + open_with(pid, 0) + } + + fn open_with(pid: u32, access: u32) -> Option { // SAFETY: plain call; a non-null result is a process handle owned by the caller. - let handle = unsafe { OpenProcess(PROCESS_SYNCHRONIZE | PROCESS_QUERY_LIMITED_INFORMATION, 0, pid) }; + let handle = unsafe { OpenProcess(PROCESS_SYNCHRONIZE | PROCESS_QUERY_LIMITED_INFORMATION | access, 0, pid) }; // SAFETY: as above. (!handle.is_null()).then(|| unsafe { OwnedHandle::from_raw_handle(handle) }) } diff --git a/crates/ide-dap/src/lib.rs b/crates/ide-dap/src/lib.rs index fea80fbe3..687742642 100644 --- a/crates/ide-dap/src/lib.rs +++ b/crates/ide-dap/src/lib.rs @@ -1130,6 +1130,9 @@ impl DebugManager { /// Ends a session: `terminate` (when asked and supported) → `disconnect` → kill. pub(crate) async fn shutdown(&self, s: &Arc, terminate: bool) { + if terminate { + crate::session::remember_debuggee(s).await; + } // java-debug runs the program itself and pipes its stdout over DAP. A `disconnect` // ends that DAP session, so every line the program writes while shutting down is lost. // Ask the program to exit ourselves and keep the adapter attached until it has drained diff --git a/crates/ide-dap/src/session.rs b/crates/ide-dap/src/session.rs index 0acc18901..d3863a300 100644 --- a/crates/ide-dap/src/session.rs +++ b/crates/ide-dap/src/session.rs @@ -59,6 +59,8 @@ pub(crate) struct Session { /// adapter may report the launcher shell, whose child is the program). #[cfg_attr(not(unix), allow(dead_code))] pub stop_pids: parking_lot::Mutex>, + #[cfg(windows)] + pub stop_tree: Mutex>, /// Breakpoints replaced for run to cursor / force stepping, restored on the next stop. pub pending_restore: Mutex, /// Adapter breakpoint id → (file, index in the stored list). @@ -111,6 +113,8 @@ impl Session { stop_force: AtomicBool::new(false), debuggee_pid: AtomicI64::new(0), stop_pids: parking_lot::Mutex::new(Vec::new()), + #[cfg(windows)] + stop_tree: Mutex::new(None), pending_restore: Mutex::default(), breakpoint_ids: Mutex::new(HashMap::new()), resolved_paths: Mutex::new(HashMap::new()), @@ -298,6 +302,21 @@ impl Session { /// How long Stop waits for a launched program to exit before reporting the session ended. pub(crate) const DEBUGGEE_EXIT_WAIT: Duration = Duration::from_secs(10); +pub(crate) async fn remember_debuggee(s: &Arc) { + #[cfg(windows)] + { + let Ok(pid) = u32::try_from(s.debuggee_pid.load(Ordering::Acquire)) else { return }; + if s.request != "launch" || pid == 0 || s.stop_tree.lock().is_some() { + return; + } + if let Ok(tree) = tokio::task::spawn_blocking(move || ide_core::process::ProcessTree::capture(pid)).await { + s.stop_tree.lock().get_or_insert(tree); + } + } + #[cfg(not(windows))] + let _ = s; +} + /// The program and everything it started: an adapter may report the launcher shell, whose /// child is the debugged program, so stopping only the reported pid leaves the program alive. #[cfg(unix)] @@ -349,12 +368,10 @@ pub(crate) fn signal_debuggee(s: &Arc, force: bool) { } #[cfg(windows)] { - let mut cmd = std::process::Command::new("taskkill"); - cmd.args(["/PID", &pid.to_string(), "/T"]); - if force { - cmd.arg("/F"); + // Console programs have no external graceful signal; the adapter gets its chance first. + if force && let Some(tree) = s.stop_tree.lock().as_ref() { + tree.terminate(); } - let _ = cmd.status(); } #[cfg(not(any(unix, windows)))] let _ = (pid, force); @@ -391,7 +408,18 @@ pub(crate) async fn wait_for_debuggee(s: &Arc, terminate: bool) { tokio::time::sleep(Duration::from_millis(50)).await; } } - #[cfg(not(unix))] + #[cfg(windows)] + { + remember_debuggee(s).await; + let deadline = std::time::Instant::now() + DEBUGGEE_EXIT_WAIT; + while s.stop_tree.lock().as_ref().is_some_and(|tree| tree.is_running()) { + if s.stop_force.swap(false, Ordering::AcqRel) || std::time::Instant::now() >= deadline { + signal_debuggee(s, true); + } + tokio::time::sleep(Duration::from_millis(50)).await; + } + } + #[cfg(not(any(unix, windows)))] { let _ = pid; } diff --git a/crates/ide-dap/src/tests.rs b/crates/ide-dap/src/tests.rs index 5e490d066..5463db03c 100644 --- a/crates/ide-dap/src/tests.rs +++ b/crates/ide-dap/src/tests.rs @@ -14,6 +14,93 @@ use tokio::io::{AsyncRead, AsyncWrite, AsyncWriteExt, BufReader}; type Log = Arc>>; +#[cfg(windows)] +#[test] +#[ignore = "child process for Windows stop tests"] +fn windows_debuggee_process() { + if std::env::var_os("OPENIDEA_TEST_DEBUGGEE").is_none() { + return; + } + if std::env::var("OPENIDEA_TEST_DEBUGGEE").unwrap() == "wrapper" { + let mut command = std::process::Command::new(std::env::current_exe().unwrap()); + let mut child = ide_core::process::prepare(&mut command) + .args(["--exact", "tests::windows_debuggee_process", "--ignored", "--nocapture"]) + .env("OPENIDEA_TEST_DEBUGGEE", "leaf") + .spawn() + .unwrap(); + let _ = child.wait(); + return; + } + let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + println!("LISTENING {}", listener.local_addr().unwrap().port()); + std::thread::sleep(Duration::from_secs(60)); +} + +#[cfg(windows)] +#[tokio::test] +async fn windows_stop_releases_the_debuggees_port_before_returning() { + use std::io::BufRead; + use std::sync::atomic::Ordering; + + struct Child(std::process::Child, Option); + impl Drop for Child { + fn drop(&mut self) { + if let Some(tree) = &self.1 { + tree.terminate(); + } + let _ = self.0.kill(); + let _ = self.0.wait(); + } + } + + for (second_stop, wrapper, supports_terminate) in + [(false, false, false), (true, false, false), (true, true, false), (true, true, true)] + { + let mut command = std::process::Command::new(std::env::current_exe().unwrap()); + ide_core::process::prepare(&mut command) + .args(["--exact", "tests::windows_debuggee_process", "--ignored", "--nocapture"]) + .env("OPENIDEA_TEST_DEBUGGEE", if wrapper { "wrapper" } else { "leaf" }) + .stdout(std::process::Stdio::piped()); + let mut child = Child(command.spawn().unwrap(), None); + let port: u16 = std::io::BufReader::new(child.0.stdout.take().unwrap()) + .lines() + .map(Result::unwrap) + .find_map(|line| line.split("LISTENING ").nth(1).and_then(|p| p.parse().ok())) + .unwrap(); + let pid = child.0.id(); + child.1 = Some(ide_core::process::ProcessTree::capture(pid)); + let h = harness(Script { debuggee_pid: Some(pid), supports_terminate, ..Default::default() }); + let id = h.start(config()).await.unwrap(); + let session = h.manager.session(id).unwrap(); + while session.debuggee_pid.load(Ordering::Acquire) != i64::from(pid) { + tokio::time::sleep(Duration::from_millis(10)).await; + } + let stop = tokio::spawn({ + let manager = h.manager.clone(); + async move { manager.stop(id).await } + }); + let deadline = std::time::Instant::now() + Duration::from_secs(5); + while session.stop_tree.lock().is_none() && !stop.is_finished() { + assert!(std::time::Instant::now() < deadline, "Stop did not capture the program tree"); + tokio::time::sleep(Duration::from_millis(10)).await; + } + assert!(!stop.is_finished(), "Stop must wait for the live program"); + if wrapper { + assert!(session.stop_tree.lock().is_some()); + child.0.kill().unwrap(); + child.0.wait().unwrap(); + assert!(std::net::TcpListener::bind(("127.0.0.1", port)).is_err(), "the child still owns its port"); + } + if second_stop { + h.manager.stop(id).await.unwrap(); + } + let limit = if second_stop { 10 } else { 25 }; + tokio::time::timeout(Duration::from_secs(limit), stop).await.unwrap().unwrap().unwrap(); + assert!(child.0.try_wait().unwrap().is_some(), "Stop returned with the program alive"); + let _listener = std::net::TcpListener::bind(("127.0.0.1", port)).expect("Stop returned with its port occupied"); + } +} + #[derive(Clone)] struct Script { /// Send a `runInTerminal` reverse request when `launch` arrives.