Commit bf64c30c08
bf64c30c08e9cff055f03670c4100b0dbf6c031f
parent: 9f77ab1b00
Verified · cmc
cmc <hello@cleberg.net> · 2026-09-28 06:29 UTC
gitutil: Transport takes a cancel channel and kills its process group
Ref #256
Layout: unified · split
internal/gitutil/gitutil.go
+19 −3
| @@ -35,8 +35,10 @@ func InitBare(path, defaultBranch, hooksPath string) error { |
| 35 | // Transport streams one git transport service (upload-pack, receive-pack, |
35 | // Transport streams one git transport service (upload-pack, receive-pack, |
| 36 | // upload-archive). extraEnv entries are appended to the process environment; |
36 | // upload-archive). extraEnv entries are appended to the process environment; |
| 37 | // hooks read the GITBAY_* variables from it. maxPack caps incoming pack |
37 | // hooks read the GITBAY_* variables from it. maxPack caps incoming pack |
| 38 | // bytes on receive-pack (0 = unlimited). |
38 | // bytes on receive-pack (0 = unlimited). Closing cancel kills the service |
| 39 | func Transport(service, repoPath string, stdin io.Reader, stdout, errW io.Writer, extraEnv []string, maxPack int64) error { |
39 | // and everything it started; a push killed before its pre-receive hook |
| |
40 | // answers updates no refs. A nil cancel never fires. |
| |
41 | func Transport(service, repoPath string, stdin io.Reader, stdout, errW io.Writer, extraEnv []string, maxPack int64, cancel <-chan struct{}) error { |
| 40 | var args []string |
42 | var args []string |
| 41 | switch service { |
43 | switch service { |
| 42 | case "git-upload-pack", "git-receive-pack", "git-upload-archive": |
44 | case "git-upload-pack", "git-receive-pack", "git-upload-archive": |
| @@ -52,7 +54,21 @@ func Transport(service, repoPath string, stdin io.Reader, stdout, errW io.Writer |
| 52 | cmd.Stdin = stdin |
54 | cmd.Stdin = stdin |
| 53 | cmd.Stdout = stdout |
55 | cmd.Stdout = stdout |
| 54 | cmd.Stderr = errW |
56 | cmd.Stderr = errW |
| 55 | return cmd.Run() |
57 | ownProcessGroup(cmd) |
| |
58 | if err := cmd.Start(); err != nil { |
| |
59 | return err |
| |
60 | } |
| |
61 | finished := make(chan struct{}) |
| |
62 | go func() { |
| |
63 | select { |
| |
64 | case <-cancel: |
| |
65 | killTree(cmd) |
| |
66 | case <-finished: |
| |
67 | } |
| |
68 | }() |
| |
69 | err := cmd.Wait() |
| |
70 | close(finished) |
| |
71 | return err |
| 56 | } |
72 | } |
| 57 | |
73 | |
| 58 | // IsAncestor reports whether old is an ancestor of new in the repository at |
74 | // IsAncestor reports whether old is an ancestor of new in the repository at |
internal/gitutil/proc_other.go
added
+13
| @@ -0,0 +1,13 @@ |
| |
1 | //go:build !unix |
| |
2 | |
| |
3 | package gitutil |
| |
4 | |
| |
5 | import "os/exec" |
| |
6 | |
| |
7 | func ownProcessGroup(cmd *exec.Cmd) {} |
| |
8 | |
| |
9 | func killTree(cmd *exec.Cmd) { |
| |
10 | if cmd.Process != nil { |
| |
11 | cmd.Process.Kill() |
| |
12 | } |
| |
13 | } |
internal/gitutil/proc_unix.go
added
+26
| @@ -0,0 +1,26 @@ |
| |
1 | //go:build unix |
| |
2 | |
| |
3 | package gitutil |
| |
4 | |
| |
5 | import ( |
| |
6 | "os/exec" |
| |
7 | "syscall" |
| |
8 | ) |
| |
9 | |
| |
10 | // ownProcessGroup puts cmd in a process group of its own, so killTree |
| |
11 | // ends what it started too: receive-pack runs index-pack and the hooks. |
| |
12 | func ownProcessGroup(cmd *exec.Cmd) { |
| |
13 | if cmd.SysProcAttr == nil { |
| |
14 | cmd.SysProcAttr = &syscall.SysProcAttr{} |
| |
15 | } |
| |
16 | cmd.SysProcAttr.Setpgid = true |
| |
17 | } |
| |
18 | |
| |
19 | func killTree(cmd *exec.Cmd) { |
| |
20 | if cmd.Process == nil { |
| |
21 | return |
| |
22 | } |
| |
23 | if err := syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL); err != nil { |
| |
24 | cmd.Process.Kill() |
| |
25 | } |
| |
26 | } |
internal/gitutil/transport_test.go
added
+33
| @@ -0,0 +1,33 @@ |
| |
1 | package gitutil |
| |
2 | |
| |
3 | import ( |
| |
4 | "io" |
| |
5 | "os/exec" |
| |
6 | "strings" |
| |
7 | "testing" |
| |
8 | "time" |
| |
9 | ) |
| |
10 | |
| |
11 | // Closing cancel kills the transport; it does not wait for the client |
| |
12 | // to hang up. Stdin ends only after the kill, as a cut connection's |
| |
13 | // does, so a clean exit here would mean the kill never happened. |
| |
14 | func TestTransportCancelKillsGit(t *testing.T) { |
| |
15 | dir := t.TempDir() |
| |
16 | if out, err := exec.Command("git", "init", "-q", "--bare", dir).CombinedOutput(); err != nil { |
| |
17 | t.Fatalf("git init: %v\n%s", err, out) |
| |
18 | } |
| |
19 | in, w := io.Pipe() |
| |
20 | cancel := make(chan struct{}) |
| |
21 | errc := make(chan error, 1) |
| |
22 | go func() { errc <- Transport("git-upload-pack", dir, in, io.Discard, io.Discard, nil, 0, cancel) }() |
| |
23 | close(cancel) |
| |
24 | time.AfterFunc(500*time.Millisecond, func() { w.Close() }) |
| |
25 | select { |
| |
26 | case err := <-errc: |
| |
27 | if err == nil || !strings.Contains(err.Error(), "killed") { |
| |
28 | t.Fatalf("Transport returned %v, want the process killed", err) |
| |
29 | } |
| |
30 | case <-time.After(5 * time.Second): |
| |
31 | t.Fatal("Transport did not return after cancel") |
| |
32 | } |
| |
33 | } |
internal/sshd/sshd.go
+1 −1
| @@ -461,7 +461,7 @@ func runGit(cfg config.Config, st *store.Store, user store.User, scope string, a |
| 461 | } |
461 | } |
| 462 | } |
462 | } |
| 463 | } |
463 | } |
| 464 | if err := gitutil.Transport(service, dir, stdin, stdout, stderr, env, maxPack); err != nil { |
464 | if err := gitutil.Transport(service, dir, stdin, stdout, stderr, env, maxPack, nil); err != nil { |
| 465 | return protocol.ExitFailure |
465 | return protocol.ExitFailure |
| 466 | } |
466 | } |
| 467 | return protocol.ExitOK |
467 | return protocol.ExitOK |