diff --git a/exec_other.go b/exec_other.go new file mode 100644 index 0000000..6ef247b --- /dev/null +++ b/exec_other.go @@ -0,0 +1,12 @@ +//go:build !unix + +package getit + +import ( + "os/exec" + "time" +) + +func killProcessGroup(cmd *exec.Cmd) { + cmd.WaitDelay = 10 * time.Second +} diff --git a/exec_unix.go b/exec_unix.go new file mode 100644 index 0000000..50284b7 --- /dev/null +++ b/exec_unix.go @@ -0,0 +1,21 @@ +//go:build unix + +package getit + +import ( + "os/exec" + "syscall" + "time" +) + +// killProcessGroup makes context cancellation kill the command's whole +// process group, not just the direct child. Without it, grandchildren (e.g. +// git-remote-https) survive the kill and hold the output pipe open, blocking +// Wait indefinitely. WaitDelay bounds Wait if anything still escapes. +func killProcessGroup(cmd *exec.Cmd) { + cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} + cmd.Cancel = func() error { + return syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL) + } + cmd.WaitDelay = 10 * time.Second +} diff --git a/git.go b/git.go index a9b4c1a..694e690 100644 --- a/git.go +++ b/git.go @@ -45,6 +45,7 @@ func (g *Git) Fetch(ctx context.Context, source Source, dest string) error { args = append(args, repoURL, dest) cmd := exec.CommandContext(ctx, "git", args...) + killProcessGroup(cmd) if output, err := cmd.CombinedOutput(); err != nil { argsStr := shellquote.Join(args...) return fmt.Errorf("git clone failed: git %s: %w: %s", argsStr, err, output) diff --git a/git_test.go b/git_test.go index 4573e61..865a317 100644 --- a/git_test.go +++ b/git_test.go @@ -6,7 +6,9 @@ import ( "os" "os/exec" "path/filepath" + "runtime" "testing" + "time" "github.com/alecthomas/assert/v2" ) @@ -96,7 +98,7 @@ func createTestRepo(t *testing.T) (repoDir string, runGit func(args ...string)) assert.NoError(t, err, "git %v failed: %s", args, output) } - runGit("init") + runGit("init", "--initial-branch=master") runGit("config", "user.email", "test@test.com") runGit("config", "user.name", "Test") @@ -202,6 +204,34 @@ func TestGitFetchCancelledContext(t *testing.T) { assert.Error(t, err) } +func TestGitFetchCancelKillsProcessGroup(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("requires sh") + } + + // Fake git that spawns a grandchild holding the output pipe open. If + // cancellation only kills the direct child, Fetch blocks until the + // grandchild exits. + binDir := t.TempDir() + script := "#!/bin/sh\nsleep 60 &\nwait\n" + assert.NoError(t, os.WriteFile(filepath.Join(binDir, "git"), []byte(script), 0o755)) //nolint:gosec + t.Setenv("PATH", binDir+string(os.PathListSeparator)+os.Getenv("PATH")) + + ctx, cancel := context.WithCancel(context.Background()) + go func() { + time.Sleep(200 * time.Millisecond) + cancel() + }() + + u, err := url.Parse("git+https://example.com/user/repo") + assert.NoError(t, err) + + start := time.Now() + err = NewGit().Fetch(ctx, Source{URL: u}, t.TempDir()) + assert.Error(t, err) + assert.True(t, time.Since(start) < 5*time.Second, "Fetch blocked on an orphaned grandchild for %s", time.Since(start)) +} + func TestGitFetchInvalidRepo(t *testing.T) { u, err := url.Parse("git+file:///nonexistent/repo/path") assert.NoError(t, err)