fix(process): reap entire process group on cmd.Cancel (#39143)

Signed-off-by: Royce Remer <royceremer@gmail.com>
Co-authored-by: wxiaoguang <wxiaoguang@gmail.com>

Author: rremer

.golangci.yml

@@ -62,6 +62,10 @@ linters:
               desc: "migrations must not depend on the models package. HINT: MIGRATION-STRUCT-FROZEN"
             - pkg: gitea.dev/modules/structs
               desc: "migrations must not depend on modules/structs. HINT: MIGRATION-STRUCT-FROZEN"
+    forbidigo:
+      forbid:
+        - pattern: '^(fmt\.Print(|f|ln)|print|println)$' # default
+        - pattern: '^exec\.CommandContext$' # use our wrapper for graceful termination
     modernize:
       disable:
         - embedlit
@@ -140,6 +144,7 @@ linters:
       - linters:
           - dupl
           - errcheck
+          - forbidigo
           - staticcheck
           - unparam
         path: _test\.go

cmd/serv.go

@@ -9,7 +9,6 @@ import (
 	"fmt"
 	"net/url"
 	"os"
-	"os/exec"
 	"path/filepath"
 	"strconv"
 	"strings"
@@ -294,7 +293,7 @@ func runServ(ctx context.Context, c *cli.Command) error {
 		return nil
 	}
 
-	var command *exec.Cmd
+	var command *process.Cmd
 	gitBinPath := filepath.Dir(gitcmd.GitExecutable) // e.g. /usr/bin
 	gitBinVerb := filepath.Join(gitBinPath, verb)    // e.g. /usr/bin/git-upload-pack
 	if _, err := os.Stat(gitBinVerb); err != nil {
@@ -303,15 +302,14 @@ func runServ(ctx context.Context, c *cli.Command) error {
 		verbFields := strings.SplitN(verb, "-", 2)
 		if len(verbFields) == 2 {
 			// use git binary with the sub-command part: "C:\...\bin\git.exe", "upload-pack", ...
-			command = exec.CommandContext(ctx, gitcmd.GitExecutable, verbFields[1], results.RepoStoragePath)
+			command = process.CommandContext(ctx, gitcmd.GitExecutable, verbFields[1], results.RepoStoragePath)
 		}
 	}
 	if command == nil {
 		// by default, use the verb (it has been checked above by allowedCommands)
-		command = exec.CommandContext(ctx, gitBinVerb, results.RepoStoragePath)
+		command = process.CommandContext(ctx, gitBinVerb, results.RepoStoragePath)
 	}
 
-	process.SetSysProcAttribute(command)
 	command.Dir = setting.RepoRootPath
 	command.Stdout = os.Stdout
 	command.Stdin = os.Stdin

modules/git/gitcmd/command.go

@@ -11,7 +11,6 @@ import (
 	"fmt"
 	"io"
 	"os"
-	"os/exec"
 	"path/filepath"
 	"strings"
 	"time"
@@ -47,7 +46,7 @@ type Command struct {
 	// otherwise some git commands might overwrite git dir internal files by a repo file.
 	gitDir string
 
-	cmd *exec.Cmd
+	cmd *process.Cmd
 
 	cmdCtx       context.Context
 	cmdCancel    process.CancelCauseFunc
@@ -432,30 +431,24 @@ func (c *Command) Start(ctx context.Context) (retErr error) {
 
 	c.cmdStartTime = time.Now()
 
-	c.cmd = exec.CommandContext(c.cmdCtx, c.prog, append(c.configArgs, c.args...)...)
+	c.cmd = process.CommandContext(c.cmdCtx, c.prog, append(c.configArgs, c.args...)...)
 	if c.cmdEnv == nil {
 		c.cmd.Env = os.Environ()
 	} else {
 		c.cmd.Env = c.cmdEnv
 	}
 
-	process.SetSysProcAttribute(c.cmd)
 	c.cmd.Env = append(c.cmd.Env, CommonGitCmdEnvs()...)
 	c.cmd.Dir = c.gitDir
 	c.cmd.Stdout = c.cmdStdout
 	c.cmd.Stdin = c.cmdStdin
 	c.cmd.Stderr = c.cmdStderr
-	c.cmd.Cancel = func() error {
-		// Golang's default cmd.Cancel only calls Process.Kill(), but here we need to close the parent pipes together:
-		// * for some commands like "git --batch-xxx", Windows git might have 2 processes (a wrapper and a real git process)
-		// * on Windows, if parent process is killed (context canceled), the children process won't be killed, and the pipe handles are still open.
-		// * if we don't close the parent pipes here, the children process won't exit.
-		//
-		// There is no such problem on POSIX, while it won't make things worse by closing the parent pipes also on POSIX.
-		err := c.cmd.Process.Kill()
+	c.cmd.WithOnCancelGracefully(func() error {
+		// Need to close the pipes to notify all sub processes to exit.
+		// Especially on Windows: there is no process group, and we didn't implement process job object (like process group).
 		c.closePipeFiles(c.parentPipeFiles)
-		return err
-	}
+		return nil
+	})
 	return c.cmd.Start()
 }
 

modules/git/gpg.go

@@ -27,7 +27,7 @@ type CommitSignSettings struct {
 	cachedPublicKeyContent atomic.Pointer[string]
 }
 
-func (css *CommitSignSettings) PublicKeyContent() (string, error) {
+func (css *CommitSignSettings) PublicKeyContent(ctx context.Context) (string, error) {
 	cached := css.cachedPublicKeyContent.Load()
 	if cached != nil {
 		return *cached, nil
@@ -43,7 +43,7 @@ func (css *CommitSignSettings) PublicKeyContent() (string, error) {
 		return s, nil
 	}
 
-	content, stderr, err := process.GetManager().Exec("gpg -a --export", "gpg", "-a", "--export", css.KeyID)
+	content, stderr, err := process.CommandContext(ctx, "gpg", "-a", "--export", css.KeyID).OutputString()
 	if err != nil {
 		return "", fmt.Errorf("unable to get default signing key: %s, %s, %w", css.KeyID, stderr, err)
 	}

modules/graceful/manager.go

@@ -11,7 +11,6 @@ import (
 
 	"gitea.dev/modules/gtprof"
 	"gitea.dev/modules/log"
-	"gitea.dev/modules/process"
 	"gitea.dev/modules/setting"
 )
 
@@ -62,9 +61,6 @@ func InitManager(ctx context.Context) {
 func initManager(ctx context.Context) {
 	initOnce.Do(func() {
 		manager = newGracefulManager(ctx)
-
-		// Set the process default context to the HammerContext
-		process.DefaultContext = manager.HammerContext()
 	})
 }
 

modules/markup/external/external.go

@@ -9,7 +9,6 @@ import (
 	"fmt"
 	"io"
 	"os"
-	"os/exec"
 	"strings"
 
 	"gitea.dev/modules/markup"
@@ -147,7 +146,7 @@ func (p *Renderer) Render(ctx *markup.RenderContext, input io.Reader, output io.
 	processCtx, _, finished := process.GetManager().AddContext(ctx, fmt.Sprintf("Render [%s] for %s", cmdProg, baseLinkSrc))
 	defer finished()
 
-	cmd := exec.CommandContext(processCtx, cmdProg, cmdArgs...)
+	cmd := process.CommandContext(processCtx, cmdProg, cmdArgs...)
 	cmd.Env = append(
 		os.Environ(),
 		"GITEA_PREFIX_SRC="+baseLinkSrc,
@@ -159,7 +158,6 @@ func (p *Renderer) Render(ctx *markup.RenderContext, input io.Reader, output io.
 	var stderr bytes.Buffer
 	cmd.Stdout = output
 	cmd.Stderr = &stderr
-	process.SetSysProcAttribute(cmd)
 
 	if err := cmd.Run(); err != nil {
 		return fmt.Errorf("%s render run command %s %v failed: %w\nStderr: %s", p.Name(), cmdProg, shellquote.Join(cmdArgs...), err, stderr.String())

modules/process/command.go

@@ -0,0 +1,54 @@
+// Copyright 2026 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package process
+
+import (
+	"bytes"
+	"context"
+	"os/exec"
+)
+
+type Cmd struct {
+	*exec.Cmd
+
+	onCancelUserFunc func() error
+	termGraceful     bool
+}
+
+func (c *Cmd) WithOnCancelGracefully(userFunc func() error) *Cmd {
+	c.termGraceful, c.onCancelUserFunc = true, userFunc
+	return c
+}
+
+func (c *Cmd) WithOnCancelForceKill(userFunc func() error) *Cmd {
+	c.termGraceful, c.onCancelUserFunc = false, userFunc
+	return c
+}
+
+func (c *Cmd) WithDir(dir string) *Cmd {
+	c.Cmd.Dir = dir
+	return c
+}
+
+func (c *Cmd) OutputString() (string, string, error) {
+	stdout, stderr := &bytes.Buffer{}, &bytes.Buffer{}
+	c.Cmd.Stdout = stdout
+	c.Cmd.Stderr = stderr
+	err := c.Cmd.Run()
+	return stdout.String(), stderr.String(), err
+}
+
+// CommandContext returns a wrapped exec.Cmd which kills the process group when the context is canceled.
+// By default, it uses graceful termination (SIGTERM) on Unix-like systems to make the subprocesses have chances
+// to clean up (e.g. remove temporary files or lock files).
+func CommandContext(ctx context.Context, name string, arg ...string) *Cmd {
+	c := &Cmd{Cmd: exec.CommandContext(ctx, name, arg...)} //nolint:forbidigo // wrap it
+	setSysProcAttribute(c.Cmd)
+	c.Cmd.Cancel = c.onCancel
+
+	// Unlike exec.CommandContext, we use graceful termination by default to avoid corrupting data or leaving lock files behind.
+	// If some processes don't respond to SIGTERM, can switch to WithOnCancelForceKill (SIGKILL) to force kill them.
+	c.termGraceful = true
+	return c
+}

modules/process/command_unix.go

@@ -0,0 +1,32 @@
+// Copyright 2026 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+//go:build unix
+
+package process
+
+import (
+	"os/exec"
+	"syscall"
+
+	"gitea.dev/modules/util"
+)
+
+func setSysProcAttribute(cmd *exec.Cmd) {
+	// When Gitea runs SubProcessA -> SubProcessB and SubProcessA gets killed by context cancel,
+	// use process group to make sure the sub processes can be killed and reaped instead of leaving defunct(zombie) processes.
+	cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}
+}
+
+func (c *Cmd) onCancel() error {
+	if c.onCancelUserFunc != nil {
+		if err := c.onCancelUserFunc(); err != nil {
+			return err
+		}
+	}
+	sig := util.Iif(c.termGraceful, syscall.SIGTERM, syscall.SIGKILL)
+	// kill the whole process group
+	// ATTENTION: do not access PID after Wait or in other goroutine, it will just cause PID reuse data-race.
+	// There is no easy solution to implement "first SIGTERM then SIGKILL" in a safe way, only one signal can be sent to the process group.
+	return syscall.Kill(-c.Process.Pid, sig)
+}

modules/process/command_unix_test.go

@@ -0,0 +1,67 @@
+// Copyright 2026 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+//go:build unix
+
+package process
+
+import (
+	"bufio"
+	"context"
+	"os"
+	"strconv"
+	"strings"
+	"syscall"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+func TestCommandContextCancelKillProcessGroup(t *testing.T) {
+	ctx, cancel := context.WithCancel(t.Context())
+	defer cancel()
+
+	// When executing external commands, there can be multiple subprocesses involved.
+	// e.g.: Gitea ->  "git upload-pack" -> "git pack-objects".
+	// If the context is canceled (HTTP client disconnects), all subprocesses must terminate.
+
+	// Spawn a shell that itself spawns a long-lived background process and
+	// prints its PID — mimicking git upload-pack spawning git pack-objects.
+
+	r, w, err := os.Pipe()
+	require.NoError(t, err)
+
+	cmd := CommandContext(ctx, "sh", "-c", "sleep 600 & echo $!; wait")
+	cmd.Stdout = w
+	require.NoError(t, cmd.Start())
+	_ = w.Close() // parent keeps only the read end
+
+	t.Cleanup(func() {
+		// make sure our test doesn't leave a zombie process even if test fails
+		_ = syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL)
+		_ = cmd.Wait()
+	})
+
+	// Block until the shell prints the grandchild PID.
+	scanner := bufio.NewScanner(r)
+	require.True(t, scanner.Scan(), "expected grandchild PID on stdout")
+	grandchildPID, err := strconv.Atoi(strings.TrimSpace(scanner.Text()))
+	require.NoError(t, err)
+	_ = r.Close()
+
+	// Sanity: grandchild must be alive before we cancel.
+	grandchild, err := os.FindProcess(grandchildPID)
+	require.NoError(t, err)
+	require.NoError(t, grandchild.Signal(syscall.Signal(0)), "grandchild should be alive before cancel")
+
+	// Cancel the context
+	cancel()
+	_ = cmd.Wait()
+
+	// Subprocess should not exist after context cancel (killed by process group)
+	assert.Eventually(t, func() bool {
+		return grandchild.Signal(syscall.Signal(0)) != nil
+	}, 5*time.Second, 10*time.Millisecond)
+}

modules/process/command_windows.go

@@ -0,0 +1,19 @@
+// Copyright 2026 The Gitea Authors. All rights reserved.
+// SPDX-License-Identifier: MIT
+
+package process
+
+import "os/exec"
+
+// There is no graceful way to kill a process on Windows at the moment
+
+func setSysProcAttribute(cmd *exec.Cmd) {}
+
+func (c *Cmd) onCancel() error {
+	if c.onCancelUserFunc != nil {
+		if err := c.onCancelUserFunc(); err != nil {
+			return err
+		}
+	}
+	return c.Process.Kill()
+}