Skip process.kill() when returncode is already set so generic exception
handlers after a successful communicate() cannot raise ProcessLookupError.
Suppress race kill failures and still run the safety-net reap.
Add unit and integration coverage for owned-PID reaping on normal exit,
timeout, exception, and exec-session kill/poll paths.
A global reaper can race asyncio's child watcher for just-exited
create_subprocess_exec children, causing Process returncode 255 instead
of the real command status. Keep only owned-PID reaps after wait/communicate
/kill paths in shell and exec_session.
Addresses review feedback on #4840.
Windows CI runs Unix-path unit tests by patching _IS_WINDOWS=False while
still on win32, where os.WNOHANG/os.waitpid do not exist. Use capability
checks so reaping is a no-op on platforms without waitpid rather than
trusting the (mockable) platform flag.
The previous fix (dbcc7cb5) only added os.waitpid() to _kill_process(),
covering the timeout/cancel path of one-shot exec. Zombies continued to
accumulate because several other exit paths never reaped children:
- _ExecSession.kill(): sent SIGKILL + process.wait(5s) but had no
os.waitpid() fallback if the wait timed out
- ExecTool.execute() generic exception handler: leaked the subprocess
if communicate() raised an unexpected error
- Normal completion paths: relied entirely on asyncio's child-watcher,
which can miss exits inside Docker containers (pidfd/SIGCHLD gaps)
Changes:
- Extract _reap_pid() helper for consistent, safe os.waitpid(WNOHANG)
- Add _reap_pid() fallback to _ExecSession.kill() via try/finally
- Add _reap_pid() safety-net after normal process exit in both
ExecTool.execute() and _ExecSession.poll()
- Kill + reap subprocess in the generic except Exception handler
- Add periodic zombie reaper background task (every 30s) in the
gateway as a last line of defense
Eric YangandXubin Ren