diff --git a/ext/git/client.go b/ext/git/client.go index e2270ca1a..db18f086e 100644 --- a/ext/git/client.go +++ b/ext/git/client.go @@ -13,7 +13,6 @@ import ( "sort" "strconv" "strings" - "syscall" "time" "github.com/bmatcuk/doublestar/v4" @@ -806,13 +805,10 @@ func (m *nativeGitClient) runCmdOutput(cmd *exec.Cmd, ropts runOpts) (string, er // Run git in its own process group so that child processes (e.g. git-remote-https) // can be cleaned up when the parent is killed on timeout or context cancellation. - cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} + setSysProcAttr(cmd) opts := executil.ExecRunOpts{ - TimeoutBehavior: executil.TimeoutBehavior{ - Signal: syscall.SIGTERM, - ShouldWait: true, - }, + TimeoutBehavior: newTimeoutBehavior(), SkipErrorLogging: ropts.SkipErrorLogging, CaptureStderr: ropts.CaptureStderr, } @@ -820,15 +816,8 @@ func (m *nativeGitClient) runCmdOutput(cmd *exec.Cmd, ropts runOpts) (string, er // After the git command finishes (normally or via timeout/context cancellation), // kill the entire process group to clean up any orphaned child processes such as - // git-remote-https. Without this, child processes accumulate over time and - // eventually exhaust the container's PID limit, causing "cannot fork()" errors. - // - // The negative int `-cmd.Process.Pid` denotes the process group id, which was set - // to PID in `SysProcAttr{Setpgid: true}` earlier. The `syscall.Kill()` below will - // send SIGKILL to all processes in the group. - if cmd.Process != nil { - _ = syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL) - } + // git-remote-https. + killProcessGroup(cmd) return output, err } diff --git a/ext/git/procattr_unix.go b/ext/git/procattr_unix.go new file mode 100644 index 000000000..43e1a4e5b --- /dev/null +++ b/ext/git/procattr_unix.go @@ -0,0 +1,34 @@ +//go:build !windows + +package git + +import ( + "os/exec" + "syscall" + + executil "github.com/argoproj/argo-cd/v3/util/exec" +) + +// setSysProcAttr configures the command to run in its own process group so that +// child processes (e.g. git-remote-https) can be cleaned up when the parent is +// killed on timeout or context cancellation. +func setSysProcAttr(cmd *exec.Cmd) { + cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} +} + +// killProcessGroup kills the entire process group to clean up any orphaned +// child processes such as git-remote-https. The negative PID denotes the +// process group, which was set via Setpgid above. +func killProcessGroup(cmd *exec.Cmd) { + if cmd.Process != nil { + _ = syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL) + } +} + +// newTimeoutBehavior returns the platform-specific timeout behavior. +func newTimeoutBehavior() executil.TimeoutBehavior { + return executil.TimeoutBehavior{ + Signal: syscall.SIGTERM, + ShouldWait: true, + } +} diff --git a/ext/git/procattr_windows.go b/ext/git/procattr_windows.go new file mode 100644 index 000000000..27a8d4a4e --- /dev/null +++ b/ext/git/procattr_windows.go @@ -0,0 +1,26 @@ +//go:build windows + +package git + +import ( + "os/exec" + "syscall" + + executil "github.com/argoproj/argo-cd/v3/util/exec" +) + +// setSysProcAttr is a no-op on Windows because Setpgid is not supported. +func setSysProcAttr(_ *exec.Cmd) {} + +// killProcessGroup is a no-op on Windows because syscall.Kill with negative +// PIDs (process groups) is not supported. +func killProcessGroup(_ *exec.Cmd) {} + +// newTimeoutBehavior returns the platform-specific timeout behavior. +// On Windows, SIGKILL is the only reliable signal, and we don't wait. +func newTimeoutBehavior() executil.TimeoutBehavior { + return executil.TimeoutBehavior{ + Signal: syscall.SIGKILL, + ShouldWait: false, + } +}