From da078a6c7514389260e8a00e9179441b71fa5489 Mon Sep 17 00:00:00 2001 From: j-zhangyiyuan Date: Mon, 3 Aug 2026 10:08:03 +0800 Subject: [PATCH] fix: terminate owned runtime process tree on stop/forceStop MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a private kill-process-tree helper to each SDK, called from the existing owned-process termination points in stop() and forceStop(). Spawn-time isolation (POSIX): - Node.js: detached: true - Python: start_new_session=True - Go: SysProcAttr.Setpgid = true - Rust: process_group(0) Teardown: - Windows (all): taskkill /T /F /PID - Node.js/Python/Go (POSIX): kill(-pid, SIGKILL) — process group signal - Rust (POSIX): libc::kill(-pid, SIGKILL) - Java: ProcessHandle.descendants() snapshot + destroyForcibly each - .NET: already uses Kill(entireProcessTree: true) — no change needed No public API changes. External-server and in-process (FFI) paths are not affected. Closes #1804 --- go/client.go | 10 +- go/process_other.go | 16 +- go/process_windows.go | 9 +- .../com/github/copilot/CopilotClient.java | 36 +++- nodejs/src/client.ts | 60 ++++++- nodejs/test/process_tree_kill.test.ts | 168 ++++++++++++++++++ python/copilot/client.py | 45 ++++- python/test_client.py | 85 +++++++++ rust/src/lib.rs | 88 +++++++-- 9 files changed, 487 insertions(+), 30 deletions(-) create mode 100644 nodejs/test/process_tree_kill.test.ts diff --git a/go/client.go b/go/client.go index fb02897f9..7823237a8 100644 --- a/go/client.go +++ b/go/client.go @@ -656,7 +656,9 @@ func (c *Client) ForceStop() { // Kill the process without waiting for startStopMux, which Start may hold. // This unblocks any I/O Start is doing (connect, version check). if p := c.osProcess.Swap(nil); p != nil { - p.Kill() + if err := killProcessTreeByPid(p.Pid); err != nil { + p.Kill() + } } // Clear sessions immediately without trying to destroy them @@ -2231,8 +2233,10 @@ func (c *Client) killProcess() error { c.ffiHost = nil } if p := c.osProcess.Swap(nil); p != nil { - if err := p.Kill(); err != nil { - return fmt.Errorf("failed to kill CLI process: %w", err) + if err := killProcessTreeByPid(p.Pid); err != nil { + if killErr := p.Kill(); killErr != nil { + return fmt.Errorf("failed to kill CLI process: %w", killErr) + } } } c.process = nil diff --git a/go/process_other.go b/go/process_other.go index 5b3ba6353..1e64ddc25 100644 --- a/go/process_other.go +++ b/go/process_other.go @@ -2,10 +2,18 @@ package copilot -import "os/exec" +import ( + "os/exec" + "syscall" +) -// configureProcAttr configures platform-specific process attributes. -// On non-Windows platforms, this is a no-op. +// configureProcAttr places the runtime in its own process group so +// killProcessTreeByPid can signal all descendants atomically. func configureProcAttr(cmd *exec.Cmd) { - // No special configuration needed on non-Windows platforms + cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} +} + +// killProcessTreeByPid signals the process group (negative PID) with SIGKILL. +func killProcessTreeByPid(pid int) error { + return syscall.Kill(-pid, syscall.SIGKILL) } diff --git a/go/process_windows.go b/go/process_windows.go index 37f954fca..cab164e89 100644 --- a/go/process_windows.go +++ b/go/process_windows.go @@ -3,14 +3,19 @@ package copilot import ( + "fmt" "os/exec" "syscall" ) -// configureProcAttr configures platform-specific process attributes. -// On Windows, this hides the console window to avoid distracting users in GUI apps. +// configureProcAttr hides the console window on Windows. func configureProcAttr(cmd *exec.Cmd) { cmd.SysProcAttr = &syscall.SysProcAttr{ HideWindow: true, } } + +// killProcessTreeByPid terminates the entire process tree via taskkill /T /F. +func killProcessTreeByPid(pid int) error { + return exec.Command("taskkill", "/T", "/F", "/PID", fmt.Sprintf("%d", pid)).Run() +} diff --git a/java/sdk/src/main/java/com/github/copilot/CopilotClient.java b/java/sdk/src/main/java/com/github/copilot/CopilotClient.java index cdd1b9ff3..154e5889f 100644 --- a/java/sdk/src/main/java/com/github/copilot/CopilotClient.java +++ b/java/sdk/src/main/java/com/github/copilot/CopilotClient.java @@ -812,19 +812,19 @@ private static void cleanupCliProcess(Process process, boolean forceImmediately) // will never come just wastes time, so terminate the child // immediately and only wait to reap it. if (forceImmediately) { - process.destroyForcibly(); + killProcessTree(process, true); if (!process.waitFor(FORCE_KILL_TIMEOUT_SECONDS, TimeUnit.SECONDS)) { LOG.fine("Process did not terminate within force kill timeout"); } return; } - process.destroy(); + killProcessTree(process, false); if (process.waitFor(FORCE_KILL_TIMEOUT_SECONDS, TimeUnit.SECONDS)) { return; } - process.destroyForcibly(); + killProcessTree(process, true); if (!process.waitFor(FORCE_KILL_TIMEOUT_SECONDS, TimeUnit.SECONDS)) { LOG.fine("Process did not terminate within force kill timeout"); } @@ -837,6 +837,36 @@ private static void cleanupCliProcess(Process process, boolean forceImmediately) } } + /** + * Terminates the runtime's process tree, ending the descendants before the root + * so none of them are reparented and left behind. + * + * @param process + * the runtime process + * @param force + * {@code true} to terminate forcibly, {@code false} to request a + * graceful exit first + */ + private static void killProcessTree(Process process, boolean force) { + try { + // descendants() is empty once the root is gone, so collect first. + process.toHandle().descendants().toList().forEach(ph -> { + if (force) { + ph.destroyForcibly(); + } else { + ph.destroy(); + } + }); + } catch (Exception e) { + LOG.log(Level.FINE, "Error terminating process descendants", e); + } + if (force) { + process.destroyForcibly(); + } else { + process.destroy(); + } + } + /** * Creates a new Copilot session with the specified configuration. *

diff --git a/nodejs/src/client.ts b/nodejs/src/client.ts index 30095186e..013826a08 100644 --- a/nodejs/src/client.ts +++ b/nodejs/src/client.ts @@ -11,7 +11,7 @@ * @module client */ -import { spawn, type ChildProcess } from "node:child_process"; +import { spawn, execSync, type ChildProcess } from "node:child_process"; import { randomUUID } from "node:crypto"; import { existsSync } from "node:fs"; import { createRequire } from "node:module"; @@ -153,6 +153,42 @@ async function waitForChildExit(child: ChildProcess, timeoutMs: number): Promise }); } +/** + * Terminate the runtime's process tree. + * + * - Windows: `taskkill /T /F` kills the entire tree rooted at `pid`. The signal + * is ignored because Windows has no graceful equivalent and `/T` can only + * enumerate the tree while the root is still alive. + * - POSIX: the runtime is spawned in its own process group (`detached: true`), + * so `kill(-pid, signal)` signals every process in that group. + * + * Falls back to `child.kill(signal)` whenever the tree-wide path is unavailable + * or fails, so behaviour degrades to the single-process termination it replaced. + * + * @see https://github.com/github/copilot-sdk/issues/1804 + */ +function killProcessTree(child: ChildProcess, signal: NodeJS.Signals = "SIGTERM"): boolean { + const pid = child.pid; + if (pid == null) { + return child.kill(signal); + } + if (process.platform === "win32") { + try { + execSync(`taskkill /T /F /PID ${pid}`, { stdio: "ignore", timeout: 5000 }); + return true; + } catch { + return child.kill(signal); + } + } + // POSIX: signal the process group (negative PID). + try { + process.kill(-pid, signal); + return true; + } catch { + return child.kill(signal); + } +} + /** * Convert tool parameters to JSON schema format for sending to CLI */ @@ -1104,8 +1140,13 @@ export class CopilotClient { this.cliProcess = null; try { if (child.exitCode == null && child.signalCode == null) { - child.kill(); - if (!(await waitForChildExit(child, RUNTIME_SHUTDOWN_TIMEOUT_MS))) { + killProcessTree(child, "SIGTERM"); + const rootExited = await waitForChildExit(child, RUNTIME_SHUTDOWN_TIMEOUT_MS); + // The root exiting says nothing about descendants that ignored + // SIGTERM, and they are the orphans this is meant to prevent, so + // sweep the group either way. + killProcessTree(child, "SIGKILL"); + if (!rootExited && !(await waitForChildExit(child, RUNTIME_SHUTDOWN_TIMEOUT_MS))) { errors.push( new Error( `Timed out waiting for CLI process to exit after kill: ${RUNTIME_SHUTDOWN_TIMEOUT_MS}ms` @@ -1231,7 +1272,7 @@ export class CopilotClient { // Force kill CLI process (only if we spawned it) if (this.cliProcess && !this.isExternalServer) { try { - this.cliProcess.kill("SIGKILL"); + killProcessTree(this.cliProcess, "SIGKILL"); } catch { // Ignore errors } @@ -2510,6 +2551,10 @@ export class CopilotClient { : ["ignore", "pipe", "pipe"]; // For .js files, spawn node explicitly; for executables, spawn directly + // Place the runtime in its own process group so killProcessTree() + // can signal all descendants atomically. On Windows detached has + // no effect — taskkill /T handles tree termination instead. + const detached = process.platform !== "win32"; const isJsFile = this.resolvedCliPath.endsWith(".js"); if (isJsFile) { this.cliProcess = spawn(getNodeExecPath(), [this.resolvedCliPath, ...args], { @@ -2517,6 +2562,7 @@ export class CopilotClient { cwd: this.options.workingDirectory, env: envWithoutNodeDebug, windowsHide: true, + detached, }); } else { this.cliProcess = spawn(this.resolvedCliPath, args, { @@ -2524,8 +2570,14 @@ export class CopilotClient { cwd: this.options.workingDirectory, env: envWithoutNodeDebug, windowsHide: true, + detached, }); } + // Prevent the detached child from keeping the parent's event loop + // alive when the embedder exits without calling stop(). + if (detached) { + this.cliProcess.unref(); + } let stdout = ""; let resolved = false; diff --git a/nodejs/test/process_tree_kill.test.ts b/nodejs/test/process_tree_kill.test.ts new file mode 100644 index 000000000..0ae791bf9 --- /dev/null +++ b/nodejs/test/process_tree_kill.test.ts @@ -0,0 +1,168 @@ +/** + * Tests for process-tree termination on stop()/forceStop(). + * + * Each case spawns a real process tree (a runtime stand-in that forks a + * long-lived grandchild), hands it to a CopilotClient as its owned runtime, + * and drives the public teardown methods. The assertions are on the SDK's + * behaviour, so removing the tree termination fails these tests. + * + * @see https://github.com/github/copilot-sdk/issues/1804 + */ +import { describe, expect, it } from "vitest"; +import { spawn, type ChildProcess } from "node:child_process"; +import { platform } from "node:os"; +import { CopilotClient, RuntimeConnection } from "../src/index.js"; + +const isWindows = platform() === "win32"; + +function isProcessAlive(pid: number): boolean { + try { + process.kill(pid, 0); + return true; + } catch { + return false; + } +} + +function sleep(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)); +} + +/** Poll until the pid is gone, so the assertion does not race the OS. */ +async function waitForExit(pid: number, timeoutMs = 10000): Promise { + const deadline = Date.now() + timeoutMs; + while (Date.now() < deadline) { + if (!isProcessAlive(pid)) { + return true; + } + await sleep(50); + } + return !isProcessAlive(pid); +} + +/** + * Spawn a runtime stand-in that forks a long-lived grandchild, matching the + * SDK's own spawn flags so the group/tree shape is the real one. + * + * The grandchild is detached on Windows and left in the group on POSIX. Either + * way it outlives a kill aimed only at the root, which is what makes these + * tests fail if tree termination regresses to terminating the root. + * + * @param ignoreSigterm - make the grandchild survive SIGTERM, reproducing the + * descendant that outlived teardown in the linked issue. + */ +function spawnRuntimeStandIn(ignoreSigterm = false): { + parent: ChildProcess; + grandchildPid: Promise; +} { + const grandchildBody = ignoreSigterm + ? `process.on("SIGTERM", () => {}); process.stdout.write("ready"); setTimeout(() => {}, 120000);` + : `process.stdout.write("ready"); setTimeout(() => {}, 120000);`; + // The grandchild only reports its pid once it is running, otherwise a signal + // can land before its SIGTERM handler is installed and it dies by default. + const helperScript = ` + const { spawn } = require("child_process"); + const child = spawn(process.execPath, ["-e", ${JSON.stringify(grandchildBody)}], { + stdio: ["ignore", "pipe", "ignore"], + detached: ${String(isWindows)}, + }); + child.stdout.once("data", () => process.stdout.write(String(child.pid))); + child.unref(); + setTimeout(() => {}, 120000); + `; + const parent = spawn(process.execPath, ["-e", helperScript], { + stdio: ["ignore", "pipe", "ignore"], + detached: !isWindows, + }); + + const grandchildPid = new Promise((resolve, reject) => { + let data = ""; + parent.stdout!.on("data", (chunk) => { + data += chunk.toString(); + const pid = parseInt(data.trim(), 10); + if (!isNaN(pid) && pid > 0) resolve(pid); + }); + parent.once("error", reject); + setTimeout(() => reject(new Error("Timeout waiting for grandchild PID")), 5000); + }); + + return { parent, grandchildPid }; +} + +/** A client that owns `child` as its runtime, without starting a real CLI. */ +function clientOwning(child: ChildProcess): CopilotClient { + const client = new CopilotClient({ + connection: RuntimeConnection.forStdio({ path: "copilot" }), + }); + (client as unknown as { cliProcess: ChildProcess }).cliProcess = child; + (client as unknown as { isExternalServer: boolean }).isExternalServer = false; + return client; +} + +describe("process tree termination", () => { + it("stop() terminates descendants of the owned runtime", async () => { + const { parent, grandchildPid } = spawnRuntimeStandIn(); + const grandchild = await grandchildPid; + const parentPid = parent.pid!; + expect(isProcessAlive(parentPid)).toBe(true); + expect(isProcessAlive(grandchild)).toBe(true); + + const errors = await clientOwning(parent).stop(); + + expect(errors).toHaveLength(0); + expect(await waitForExit(parentPid)).toBe(true); + expect(await waitForExit(grandchild)).toBe(true); + }); + + it("forceStop() terminates descendants of the owned runtime", async () => { + const { parent, grandchildPid } = spawnRuntimeStandIn(); + const grandchild = await grandchildPid; + const parentPid = parent.pid!; + + await clientOwning(parent).forceStop(); + + expect(await waitForExit(parentPid)).toBe(true); + expect(await waitForExit(grandchild)).toBe(true); + }); + + // Windows has no SIGTERM, so the escalation this covers is POSIX-only. + it.skipIf(isWindows)( + "stop() reaps a descendant that ignores SIGTERM", + async () => { + const { parent, grandchildPid } = spawnRuntimeStandIn(true); + const grandchild = await grandchildPid; + const parentPid = parent.pid!; + + await clientOwning(parent).stop(); + + expect(await waitForExit(parentPid)).toBe(true); + expect(await waitForExit(grandchild)).toBe(true); + }, + 30000 + ); + + it("leaves processes alone for external-server connections", async () => { + const { parent, grandchildPid } = spawnRuntimeStandIn(); + const grandchild = await grandchildPid; + const parentPid = parent.pid!; + + const client = new CopilotClient({ + connection: RuntimeConnection.forUri("http://localhost:19999"), + }); + (client as unknown as { cliProcess: ChildProcess }).cliProcess = parent; + expect((client as unknown as { isExternalServer: boolean }).isExternalServer).toBe(true); + + const errors = await client.stop(); + + expect(errors).toHaveLength(0); + expect(isProcessAlive(parentPid)).toBe(true); + expect(isProcessAlive(grandchild)).toBe(true); + + parent.kill("SIGKILL"); + try { + process.kill(grandchild, "SIGKILL"); + } catch { + // Already gone. + } + }); +}); diff --git a/python/copilot/client.py b/python/copilot/client.py index 6cdd765c3..a75ad081b 100644 --- a/python/copilot/client.py +++ b/python/copilot/client.py @@ -20,6 +20,7 @@ import os import re import shutil +import signal import subprocess import sys import threading @@ -1259,6 +1260,40 @@ def _session_lifecycle_event_from_dict(data: dict) -> SessionLifecycleEvent: _CLI_PROCESS_EXIT_TIMEOUT_SECONDS = 5 +def _kill_process_tree(proc: subprocess.Popen[Any], *, force: bool = False) -> None: + """Signal the runtime's process tree. + + Windows: ``taskkill /T /F`` walks the tree rooted at *pid*. ``force`` is + ignored there because Windows has no graceful equivalent and ``/T`` can only + enumerate the tree while the root is still alive. POSIX: the runtime is + spawned with ``start_new_session=True``, so ``os.killpg`` reaches every + descendant that stayed in the group. + + Falls back to the ``Popen`` methods whenever a real pid is unavailable or + the tree-wide signal fails, so behaviour degrades to the single-process + termination this replaced. + + See: https://github.com/github/copilot-sdk/issues/1804 + """ + fallback = proc.kill if force else proc.terminate + pid = getattr(proc, "pid", None) + if not isinstance(pid, int): + fallback() + return + if sys.platform == "win32": + try: + args = ["taskkill", "/T", "/F", "/PID", str(pid)] + if subprocess.run(args, capture_output=True, timeout=5).returncode != 0: + fallback() + except Exception: + fallback() + return + try: + os.killpg(pid, signal.SIGKILL if force else signal.SIGTERM) + except (ProcessLookupError, PermissionError, OSError): + fallback() + + def _get_or_download_cli(*, include_runtime_lib: bool = False) -> str | None: """Get the cached CLI binary, downloading if necessary. @@ -1990,14 +2025,14 @@ async def stop(self) -> None: poll = getattr(self._cli_process, "poll", None) is_running = poll is None or poll() is None if is_running: - self._cli_process.terminate() + _kill_process_tree(self._cli_process) try: await asyncio.to_thread( self._cli_process.wait, timeout=_CLI_PROCESS_EXIT_TIMEOUT_SECONDS, ) except subprocess.TimeoutExpired: - self._cli_process.kill() + _kill_process_tree(self._cli_process, force=True) try: await asyncio.to_thread( self._cli_process.wait, @@ -2056,7 +2091,7 @@ async def force_stop(self) -> None: if self._process is not None and self._process is not self._cli_process: self._process.terminate() if self._cli_process is not None: - self._cli_process.kill() + _kill_process_tree(self._cli_process, force=True) self._process = None self._cli_process = None except Exception: @@ -4182,6 +4217,9 @@ async def _start_cli_server(self) -> None: cwd=cwd, env=env, creationflags=creationflags, + # Place the runtime in its own process group so + # _kill_process_tree() can signal all descendants. + start_new_session=(sys.platform != "win32"), ) self._cli_process = self._process else: @@ -4195,6 +4233,7 @@ async def _start_cli_server(self) -> None: cwd=cwd, env=env, creationflags=creationflags, + start_new_session=(sys.platform != "win32"), ) self._cli_process = self._process log_timing( diff --git a/python/test_client.py b/python/test_client.py index cf4bdf192..dd9b87991 100644 --- a/python/test_client.py +++ b/python/test_client.py @@ -7,12 +7,14 @@ import asyncio import inspect import os +import signal from datetime import UTC, datetime from tempfile import TemporaryDirectory from unittest.mock import AsyncMock, Mock, patch import pytest +import copilot.client from copilot import ( CanvasProviderIdentity, CapiSessionOptions, @@ -174,6 +176,89 @@ async def test_force_stop_external_server_clears_process_references(self): assert client._cli_process is None +class TestKillProcessTree: + """Coverage for the platform paths of the process-tree termination helper.""" + + @staticmethod + def _proc(pid=4321): + proc = Mock() + proc.pid = pid + return proc + + @staticmethod + def _as_posix(monkeypatch): + """Pretend to be POSIX, supplying the symbols Windows hosts lack.""" + monkeypatch.setattr(copilot.client.sys, "platform", "linux") + monkeypatch.setattr(copilot.client.signal, "SIGKILL", 9, raising=False) + + def test_posix_signals_the_process_group(self, monkeypatch): + proc = self._proc() + signalled: list[tuple[int, int]] = [] + self._as_posix(monkeypatch) + monkeypatch.setattr( + copilot.client.os, + "killpg", + lambda pid, sig: signalled.append((pid, sig)), + raising=False, + ) + + copilot.client._kill_process_tree(proc) + copilot.client._kill_process_tree(proc, force=True) + + assert signalled == [(4321, signal.SIGTERM), (4321, copilot.client.signal.SIGKILL)] + proc.terminate.assert_not_called() + proc.kill.assert_not_called() + + def test_posix_falls_back_when_the_group_is_gone(self, monkeypatch): + proc = self._proc() + self._as_posix(monkeypatch) + + def boom(pid, sig): + raise ProcessLookupError + + monkeypatch.setattr(copilot.client.os, "killpg", boom, raising=False) + + copilot.client._kill_process_tree(proc) + + proc.terminate.assert_called_once() + + def test_windows_uses_taskkill_for_the_whole_tree(self, monkeypatch): + proc = self._proc() + commands: list[list[str]] = [] + monkeypatch.setattr(copilot.client.sys, "platform", "win32") + monkeypatch.setattr( + copilot.client.subprocess, + "run", + lambda args, **kwargs: commands.append(args) or Mock(returncode=0), + ) + + copilot.client._kill_process_tree(proc) + + assert commands == [["taskkill", "/T", "/F", "/PID", "4321"]] + proc.terminate.assert_not_called() + + def test_windows_falls_back_when_taskkill_fails(self, monkeypatch): + proc = self._proc() + monkeypatch.setattr(copilot.client.sys, "platform", "win32") + monkeypatch.setattr( + copilot.client.subprocess, "run", lambda args, **kwargs: Mock(returncode=128) + ) + + copilot.client._kill_process_tree(proc, force=True) + + proc.kill.assert_called_once() + + def test_missing_pid_falls_back_to_the_process_handle(self, monkeypatch): + proc = Mock() # Mock.pid is not an int + monkeypatch.setattr( + copilot.client.os, "killpg", Mock(side_effect=AssertionError), raising=False + ) + + copilot.client._kill_process_tree(proc) + + proc.terminate.assert_called_once() + + class TestPermissionHandlerOptional: @pytest.mark.asyncio async def test_create_session_allows_missing_permission_handler(self): diff --git a/rust/src/lib.rs b/rust/src/lib.rs index 9e3041fec..521fa184d 100644 --- a/rust/src/lib.rs +++ b/rust/src/lib.rs @@ -1679,6 +1679,15 @@ impl Client { fn build_command(program: &Path, options: &ClientOptions, working_directory: &Path) -> Command { let mut command = Command::new(program); + + // Place the runtime in its own process group so kill_process_tree() + // can signal all descendants atomically on POSIX. + #[cfg(unix)] + { + use std::os::unix::process::CommandExt; + command.process_group(0); + } + for arg in &options.prefix_args { command.arg(arg); } @@ -2506,7 +2515,7 @@ impl Client { // response and never self-exits. Waiting for a self-exit // that will never come just wastes time, so terminate the // child immediately. - if let Err(e) = child.kill().await { + if let Err(e) = kill_process_tree(&mut child).await { errors.push(e.into()); } } @@ -2564,10 +2573,8 @@ impl Client { pub fn force_stop(&self) { let pid = self.pid(); info!(pid = ?pid, "force-stopping CLI process"); - if let Some(mut child) = self.inner.child.lock().take() - && let Err(e) = child.start_kill() - { - error!(pid = ?pid, error = %e, "failed to send kill signal"); + if let Some(mut child) = self.inner.child.lock().take() { + force_kill_process_tree(&mut child); } self.inner.rpc.force_close(); #[cfg(feature = "bundled-in-process")] @@ -2622,15 +2629,74 @@ impl Client { } } +/// Signal the runtime's whole process tree, returning whether it succeeded. +/// +/// POSIX: the runtime is spawned with `process_group(0)`, so its PID doubles as +/// the PGID and `kill -9 -` reaches every descendant still in the group. +/// Windows: `taskkill /T /F` walks the tree, which it can only do while the +/// root is alive. +fn signal_process_tree(child: &Child) -> bool { + let Some(pid) = child.id() else { + return false; + }; + #[cfg(unix)] + let mut command = { + let mut c = std::process::Command::new("kill"); + c.args(["-9", &format!("-{pid}")]); + c + }; + #[cfg(windows)] + let mut command = { + let mut c = std::process::Command::new("taskkill"); + c.args(["/T", "/F", "/PID", &pid.to_string()]); + c + }; + #[cfg(not(any(unix, windows)))] + { + let _ = pid; + return false; + } + #[cfg(any(unix, windows))] + match command + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .status() + { + Ok(status) if status.success() => true, + Ok(status) => { + debug!(%status, "process tree termination reported failure"); + false + } + Err(e) => { + debug!(error = %e, "could not run the process tree termination command"); + false + } + } +} + +/// Terminate the runtime's process tree and reap the root (async, for `stop()`). +async fn kill_process_tree(child: &mut Child) -> std::io::Result<()> { + if !signal_process_tree(child) { + child.kill().await?; + } + // The root still needs reaping even when the tree signal did the killing. + child.wait().await.map(|_| ()) +} + +/// Synchronous tree kill for `force_stop()` and `Drop`. +fn force_kill_process_tree(child: &mut Child) { + if signal_process_tree(child) { + return; + } + if let Err(e) = child.start_kill() { + error!(error = %e, "failed to send kill signal"); + } +} + impl Drop for ClientInner { fn drop(&mut self) { if let Some(ref mut child) = *self.child.lock() { - let pid = child.id(); - if let Err(e) = child.start_kill() { - error!(pid = ?pid, error = %e, "failed to kill CLI process on drop"); - } else { - info!(pid = ?pid, "kill signal sent for CLI process on drop"); - } + force_kill_process_tree(child); } #[cfg(feature = "bundled-in-process")] {