fix: kill entire CLI process tree on stop/forceStop (Windows) - #2073
fix: kill entire CLI process tree on stop/forceStop (Windows)#2073rinceyuan wants to merge 2 commits into
Conversation
On Windows, ChildProcess.kill() / Popen.terminate() only terminates the immediate process via TerminateProcess(), leaving grandchildren (copilot workers, MCP servers, etc.) orphaned. Each session cycle leaks a full process tree that accumulates until OOM. Fix: use \ askkill /T /F /PID\ on Windows to terminate the entire tree. On Unix, use os.killpg() in Python for process group cleanup. Node.js (client.ts): - Add killProcessTree() helper wrapping taskkill /T on Windows - Replace child.kill() in stop() and forceStop() with killProcessTree() Python (client.py): - Add _kill_process_tree() helper with taskkill /T (Windows) and os.killpg() (Unix) strategies - Replace self._cli_process.terminate()/kill() in stop() and force_stop() with _kill_process_tree() Tested: spawned a 2-level process tree on Windows, confirmed both parent and child are terminated after taskkill /T. Closes github#1804
|
@microsoft-github-policy-service agree company=Microsoft |
|
@stephentoub This fixes the Windows process tree leak reported in #1804. Affects both Node.js and Python SDKs — each stop()/forceStop() cycle was orphaning the CLI's child processes. The fix uses \ askkill /T\ on Windows. Manually verified on Windows 11. Happy to add the Go/.NET fixes in a follow-up if desired. |
Add CopilotClientOptions.processGroup (default: false). When enabled: - The CLI child is spawned with detached:true on Unix, making it a process group leader (PGID == PID). - killProcessTree() uses process.kill(-pid, signal) to signal the entire group atomically, reaping all descendant processes (tool subprocesses, MCP servers, shells) instead of only the immediate child. - On Windows this is a no-op — taskkill /T already handles tree kill. - The detached child is unref'd to prevent keeping the event loop alive. This enables embedders running long-lived daemon hosts to guarantee the agent's full process tree is confined and reaped on stop/crash, without relying on the OS to reparent orphans. Partially addresses github#1935
|
The way this is implemented in the PR currently won’t work because:
There is a small, coherent cross-language change we could accept: add one private “terminate owned runtime tree” operation per SDK, called from the existing owned-process termination point. Its behavior should be: POSIX also needs one small spawn-time change to place the runtime in its own process group/session. Otherwise group termination could kill the host. No public API is needed. Per language, this is approximately:
Each SDK should call that helper from the process-termination section used by This is about the smallest useful implementation across all languages and OSes:
The tests can also be narrow: start a helper process that starts one long-lived child, then verify both disappear after |
Summary
Fix process tree leak on Windows when
stop()/forceStop()terminates the CLI.On Windows,
ChildProcess.kill()(Node.js) andPopen.terminate()/kill()(Python) only terminate the immediate process viaTerminateProcess(), leaving grandchildren orphaned. Eachcreate_session()/stop()cycle leaks a full copilot process tree (~3-5 processes) that survives until idle timeout or OOM.Changes
client.ts)killProcessTree()helper: usestaskkill /T /F /PIDon Windows, falls back to standard.kill(signal)on Unixclient.py)_kill_process_tree()helper: usestaskkill /Ton Windows,os.killpg()on Unix, with fallback toproc.kill()Validation
tsc --noEmit)ast.parse)taskkill /Tkills both parent and grandchild (see test output below)parent=47632 child=27936 pre:parent alive pre:child alive taskkill OK post:parent DEAD(OK) post:child DEAD(OK)Notes
os.killpg()in Python (requires the child to be a process group leader); falls back gracefully if not applicable.Closes #1804