-
Notifications
You must be signed in to change notification settings - Fork 198
Guard against PID 0 in KillProcess and FindProcess #4400
Description
Bug description
KillProcess, FindProcess, and WaitForExit in pkg/process/ do not reject PID 0 or negative PIDs. On Unix, kill(0, sig) sends the signal to every process in the calling process's group, so KillProcess(0) would SIGTERM thv itself and any child processes sharing its process group. FindProcess(0) returns a false positive via kill(0, 0) succeeding. On Windows, PID 0 is the System Idle Process; KillProcess(0) would attempt to terminate it, and FindProcess(0) would report it as alive regardless of proxy state.
Steps to reproduce
PID 0 can reach these functions when a status file contains process_id: 0, which can occur if:
- The proxy process is killed before writing its PID (OOM,
kill -9) - Disk-full prevents the status file write
- A race condition during
thv rm+thv run(mitigated by Guard PID reset against replacement process #4325 but not eliminated for all edge cases)
Once PID 0 is in the status file:
stopProcess(inmanager.go) callsKillProcess(0), sending SIGTERM to the process groupisProxyUnhealthy(infile_status.go) callsFindProcess(0), which falsely reports the proxy as aliveWaitForExitpollsFindProcessin a loop and would spin until context timeout
Expected behavior
KillProcess, FindProcess, and WaitForExit should reject PID 0 and negative PIDs with a clear error, allowing callers to handle the condition gracefully.
Actual behavior
KillProcess(0)sends SIGTERM to the entire process group (Unix) or attempts to terminate the System Idle Process (Windows)FindProcess(0)returns(true, nil), falsely indicating a live processWaitForExitwith PID 0 spins until context cancellation
Additional context
This is a defense-in-depth fix. #4325 addressed the primary source of PID 0 appearing in status files (race during replacement), but edge cases remain (OOM, kill -9, disk full). All existing callers already handle errors from these functions gracefully, so adding the guard requires no caller changes.