-
-
Notifications
You must be signed in to change notification settings - Fork 7.1k
fix(process): reap entire process group on cmd.Cancel #39143
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from 4 commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
8686ec5
fix(process): reap entire process group on cmd.Cancel, not just the p…
rremer bc21fdd
refactor
wxiaoguang 5c8b79c
fix lint
wxiaoguang 9e336b1
fix(process): introduce command.WithWaitDelay() defaulting to 10s to …
rremer 3b7449f
fix(process): Always check ProcessState before reaping by pid
rremer b08ea8a
revert pid reuse data-race
wxiaoguang 4a84a8c
comment
wxiaoguang fb61eba
Merge branch 'main' into main-orphanage
wxiaoguang 0b260ff
add lint rule to deny exec.CommandContext
wxiaoguang 8966bb6
Merge branch 'main' into main-orphanage
wxiaoguang 555eea7
Merge branch 'main' into main-orphanage
bircni File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,61 @@ | ||
| // Copyright 2026 The Gitea Authors. All rights reserved. | ||
| // SPDX-License-Identifier: MIT | ||
|
|
||
| package process | ||
|
|
||
| import ( | ||
| "bytes" | ||
| "context" | ||
| "os/exec" | ||
| "time" | ||
| ) | ||
|
|
||
| // defaultWaitDelay is how long CommandContext waits after sending SIGTERM before escalating to SIGKILL. | ||
| const defaultWaitDelay = 10 * time.Second | ||
|
|
||
| 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) WithWaitDelay(d time.Duration) *Cmd { | ||
| c.Cmd.WaitDelay = d | ||
| 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...)} | ||
| setSysProcAttribute(c.Cmd) | ||
| c.Cmd.Cancel = c.onCancel | ||
| c.Cmd.WaitDelay = defaultWaitDelay | ||
| c.termGraceful = true | ||
| return c | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| // Copyright 2026 The Gitea Authors. All rights reserved. | ||
| // SPDX-License-Identifier: MIT | ||
|
|
||
| //go:build unix | ||
|
|
||
| package process | ||
|
|
||
| import ( | ||
| "os/exec" | ||
| "syscall" | ||
| "time" | ||
|
|
||
| "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) | ||
| if sig == syscall.SIGTERM && c.Cmd.WaitDelay > 0 { | ||
| pgid, delay := c.Process.Pid, c.Cmd.WaitDelay | ||
| go func() { | ||
| time.Sleep(delay) | ||
| _ = syscall.Kill(-pgid, syscall.SIGKILL) | ||
| }() | ||
| } | ||
| // kill the whole process group | ||
| return syscall.Kill(-c.Process.Pid, sig) | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,136 @@ | ||
| // 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) | ||
| } | ||
|
|
||
| func TestCommandContextWaitDelayOrphansGrandchild(t *testing.T) { | ||
| ctx, cancel := context.WithCancel(t.Context()) | ||
| defer cancel() | ||
|
|
||
| r, w, err := os.Pipe() | ||
| require.NoError(t, err) | ||
|
|
||
| // Shell ignores SIGTERM; grandchild inherits SIG_IGN and also survives the group SIGTERM. | ||
| cmd := CommandContext(ctx, "sh", "-c", "trap '' TERM; sleep 600 & echo $!; wait"). | ||
| WithWaitDelay(1 * time.Second) | ||
| cmd.Stdout = w | ||
| require.NoError(t, cmd.Start()) | ||
| _ = w.Close() | ||
|
|
||
| var grandchildPID int | ||
| t.Cleanup(func() { | ||
| // Kill by process group — the PGID survives the group leader's death, so this reaches orphaned sleep. | ||
| _ = syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL) | ||
| _ = cmd.Wait() | ||
| if grandchildPID > 0 { | ||
| _ = syscall.Kill(grandchildPID, syscall.SIGKILL) | ||
| } | ||
| }) | ||
|
|
||
| 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() | ||
|
|
||
| grandchild, err := os.FindProcess(grandchildPID) | ||
| require.NoError(t, err) | ||
| require.NoError(t, grandchild.Signal(syscall.Signal(0)), "grandchild should be alive before cancel") | ||
|
|
||
| cancel() | ||
| _ = cmd.Wait() // blocks ~1s until WaitDelay SIGKILL kills the direct child (shell) | ||
|
|
||
| assert.Eventually(t, func() bool { | ||
| return grandchild.Signal(syscall.Signal(0)) != nil | ||
| }, 5*time.Second, 10*time.Millisecond, "grandchild was orphaned: WaitDelay SIGKILL did not reach the process group") | ||
| } | ||
|
|
||
| func TestCommandContextSIGTERMIgnoredHangs(t *testing.T) { | ||
| ctx, cancel := context.WithCancel(t.Context()) | ||
| defer cancel() | ||
|
|
||
| cmd := CommandContext(ctx, "sh", "-c", "trap '' TERM; sleep 600").WithWaitDelay(1 * time.Second) | ||
| require.NoError(t, cmd.Start()) | ||
|
|
||
| waitDone := make(chan struct{}) | ||
| go func() { | ||
| _ = cmd.Wait() | ||
| close(waitDone) | ||
| }() | ||
|
|
||
| t.Cleanup(func() { | ||
| _ = syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL) | ||
| <-waitDone | ||
| }) | ||
|
|
||
| cancel() | ||
|
|
||
| select { | ||
| case <-waitDone: | ||
| case <-time.After(5 * time.Second): | ||
| t.Fatal("process did not exit after context cancel: SIGTERM ignored and no SIGKILL escalation occurred") | ||
| } | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.