Skip to content

Commit 4cb432e

Browse files
mhaggermhagger
committed
internal/pipe: get the package to build on Windows, too
1 parent 68e73f7 commit 4cb432e

File tree

4 files changed

+100
-55
lines changed

4 files changed

+100
-55
lines changed

‎internal/pipe/command.go‎

Lines changed: 6 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ import (
99
"os/exec"
1010
"sync/atomic"
1111
"syscall"
12-
"time"
1312

1413
"golang.org/x/sync/errgroup"
1514
)
@@ -98,11 +97,8 @@ func (s *commandStage) Start(
9897
})
9998
}
10099

101-
// Put the command in its own process group:
102-
if s.cmd.SysProcAttr == nil {
103-
s.cmd.SysProcAttr = &syscall.SysProcAttr{}
104-
}
105-
s.cmd.SysProcAttr.Setpgid = true
100+
// Put the command in its own process group, if possible:
101+
s.runInOwnProcessGroup()
106102

107103
if err := s.cmd.Start(); err != nil {
108104
return nil, err
@@ -122,53 +118,6 @@ func (s *commandStage) Start(
122118
return stdout, nil
123119
}
124120

125-
// kill is called to kill the process if the context expires. `err` is
126-
// the corresponding value of `Context.Err()`.
127-
func (s *commandStage) kill(err error) {
128-
// I believe that the calls to `syscall.Kill()` in this method are
129-
// racy. It could be that s.cmd.Wait() succeeds immediately before
130-
// this call, in which case the process group wouldn't exist
131-
// anymore. But I don't see any way to avoid this without
132-
// duplicating a lot of code from `exec.Cmd`. (`os.Cmd.Kill()` and
133-
// `os.Cmd.Signal()` appear to be race-free, but only because they
134-
// use internal synchronization. But those methods only kill the
135-
// process, not the process group, so they are not suitable here.
136-
137-
// We started the process with PGID == PID:
138-
pid := s.cmd.Process.Pid
139-
select {
140-
case <-s.done:
141-
// Process has ended; no need to kill it again.
142-
return
143-
default:
144-
}
145-
146-
// Record the `ctx.Err()`, which will be used as the error result
147-
// for this stage.
148-
s.ctxErr.Store(err)
149-
150-
// First try to kill using a relatively gentle signal so that
151-
// the processes have a chance to clean up after themselves:
152-
_ = syscall.Kill(-pid, syscall.SIGTERM)
153-
154-
// Well-behaved processes should commit suicide after the above,
155-
// but if they don't exit within 2s, murder the whole lot of them:
156-
go func() {
157-
// Use an explicit `time.Timer` rather than `time.After()` so
158-
// that we can stop it (freeing resources) promptly if the
159-
// command exits before the timer triggers.
160-
timer := time.NewTimer(2 * time.Second)
161-
defer timer.Stop()
162-
163-
select {
164-
case <-s.done:
165-
// Process has ended; no need to kill it again.
166-
case <-timer.C:
167-
_ = syscall.Kill(-pid, syscall.SIGKILL)
168-
}
169-
}()
170-
}
171-
172121
// filterCmdError interprets `err`, which was returned by `Cmd.Wait()`
173122
// (possibly `nil`), possibly modifying it or ignoring it. It returns
174123
// the error that should actually be returned to the caller (possibly
@@ -186,7 +135,10 @@ func (s *commandStage) filterCmdError(err error) error {
186135
ctxErr, ok := s.ctxErr.Load().(error)
187136
if ok {
188137
// If the process looks like it was killed by us, substitute
189-
// `ctxErr` for the process's own exit error.
138+
// `ctxErr` for the process's own exit error. Note that this
139+
// doesn't do anything on Windows, where the `Signaled()`
140+
// method isn't implemented (it is hardcoded to return
141+
// `false`).
190142
ps, ok := eErr.ProcessState.Sys().(syscall.WaitStatus)
191143
if ok && ps.Signaled() &&
192144
(ps.Signal() == syscall.SIGTERM || ps.Signal() == syscall.SIGKILL) {

‎internal/pipe/command_unix.go‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
//go:build !windows
2+
// +build !windows
3+
4+
package pipe
5+
6+
import (
7+
"syscall"
8+
"time"
9+
)
10+
11+
// runInOwnProcessGroup arranges for `cmd` to be run in its own
12+
// process group.
13+
func (s *commandStage) runInOwnProcessGroup() {
14+
// Put the command in its own process group:
15+
if s.cmd.SysProcAttr == nil {
16+
s.cmd.SysProcAttr = &syscall.SysProcAttr{}
17+
}
18+
s.cmd.SysProcAttr.Setpgid = true
19+
}
20+
21+
// kill is called to kill the process if the context expires. `err` is
22+
// the corresponding value of `Context.Err()`.
23+
func (s *commandStage) kill(err error) {
24+
// I believe that the calls to `syscall.Kill()` in this method are
25+
// racy. It could be that s.cmd.Wait() succeeds immediately before
26+
// this call, in which case the process group wouldn't exist
27+
// anymore. But I don't see any way to avoid this without
28+
// duplicating a lot of code from `exec.Cmd`. (`os.Cmd.Kill()` and
29+
// `os.Cmd.Signal()` appear to be race-free, but only because they
30+
// use internal synchronization. But those methods only kill the
31+
// process, not the process group, so they are not suitable here.
32+
33+
// We started the process with PGID == PID:
34+
pid := s.cmd.Process.Pid
35+
select {
36+
case <-s.done:
37+
// Process has ended; no need to kill it again.
38+
return
39+
default:
40+
}
41+
42+
// Record the `ctx.Err()`, which will be used as the error result
43+
// for this stage.
44+
s.ctxErr.Store(err)
45+
46+
// First try to kill using a relatively gentle signal so that
47+
// the processes have a chance to clean up after themselves:
48+
_ = syscall.Kill(-pid, syscall.SIGTERM)
49+
50+
// Well-behaved processes should commit suicide after the above,
51+
// but if they don't exit within 2s, murder the whole lot of them:
52+
go func() {
53+
// Use an explicit `time.Timer` rather than `time.After()` so
54+
// that we can stop it (freeing resources) promptly if the
55+
// command exits before the timer triggers.
56+
timer := time.NewTimer(2 * time.Second)
57+
defer timer.Stop()
58+
59+
select {
60+
case <-s.done:
61+
// Process has ended; no need to kill it again.
62+
case <-timer.C:
63+
_ = syscall.Kill(-pid, syscall.SIGKILL)
64+
}
65+
}()
66+
}

‎internal/pipe/command_windows.go‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
//go:build windows
2+
// +build windows
3+
4+
package pipe
5+
6+
// runInOwnProcessGroup is not supported on Windows.
7+
func (s *commandStage) runInOwnProcessGroup() {}
8+
9+
// kill is called to kill the process if the context expires. `err` is
10+
// the corresponding value of `Context.Err()`.
11+
func (s *commandStage) kill(err error) {
12+
select {
13+
case <-s.done:
14+
// Process has ended; no need to kill it again.
15+
return
16+
default:
17+
}
18+
19+
// Record the `ctx.Err()`, which will be used as the error result
20+
// for this stage.
21+
s.ctxErr.Store(err)
22+
23+
s.cmd.Process.Kill()
24+
}

‎internal/pipe/filter-error.go‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,10 @@ func IsError(target error) ErrorMatcher {
8787

8888
// IsSIGPIPE returns an `ErrorMatcher` that matches `*exec.ExitError`s
8989
// that were caused by the specified signal. The match for
90-
// `*exec.ExitError`s uses `errors.As()`.
90+
// `*exec.ExitError`s uses `errors.As()`. Note that under Windows this
91+
// always returns false, because on that platform
92+
// `WaitStatus.Signaled()` isn't implemented (it is hardcoded to
93+
// return `false`).
9194
func IsSignal(signal syscall.Signal) ErrorMatcher {
9295
return func(err error) bool {
9396
var eErr *exec.ExitError

0 commit comments

Comments
 (0)