Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit6f3f7f2

Browse files
authored
fix(agent): Allow signal propagation when running as PID 1 (#6141)
1 parentaf59e2b commit6f3f7f2

File tree

4 files changed

+88
-10
lines changed

4 files changed

+88
-10
lines changed

‎agent/reaper/reaper.go

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
package reaper
22

3-
import"github.com/hashicorp/go-reap"
3+
import (
4+
"os"
5+
6+
"github.com/hashicorp/go-reap"
7+
)
48

59
typeOptionfunc(o*options)
610

@@ -22,7 +26,16 @@ func WithPIDCallback(ch reap.PidCh) Option {
2226
}
2327
}
2428

29+
// WithCatchSignals sets the signals that are caught and forwarded to the
30+
// child process. By default no signals are forwarded.
31+
funcWithCatchSignals(sigs...os.Signal)Option {
32+
returnfunc(o*options) {
33+
o.CatchSignals=sigs
34+
}
35+
}
36+
2537
typeoptionsstruct {
26-
ExecArgs []string
27-
PIDs reap.PidCh
38+
ExecArgs []string
39+
PIDs reap.PidCh
40+
CatchSignals []os.Signal
2841
}

‎agent/reaper/reaper_test.go

Lines changed: 42 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,11 @@
33
package reaper_test
44

55
import (
6+
"fmt"
67
"os"
78
"os/exec"
9+
"os/signal"
10+
"syscall"
811
"testing"
912
"time"
1013

@@ -15,9 +18,8 @@ import (
1518
"github.com/coder/coder/testutil"
1619
)
1720

21+
//nolint:paralleltest // Non-parallel subtest.
1822
funcTestReap(t*testing.T) {
19-
t.Parallel()
20-
2123
// Don't run the reaper test in CI. It does weird
2224
// things like forkexecing which may have unintended
2325
// consequences in CI.
@@ -28,8 +30,9 @@ func TestReap(t *testing.T) {
2830
// OK checks that's the reaper is successfully reaping
2931
// exited processes and passing the PIDs through the shared
3032
// channel.
33+
34+
//nolint:paralleltest // Signal handling.
3135
t.Run("OK",func(t*testing.T) {
32-
t.Parallel()
3336
pids:=make(reap.PidCh,1)
3437
err:=reaper.ForkReap(
3538
reaper.WithPIDCallback(pids),
@@ -64,3 +67,39 @@ func TestReap(t *testing.T) {
6467
}
6568
})
6669
}
70+
71+
//nolint:paralleltest // Signal handling.
72+
funcTestReapInterrupt(t*testing.T) {
73+
// Don't run the reaper test in CI. It does weird
74+
// things like forkexecing which may have unintended
75+
// consequences in CI.
76+
if_,ok:=os.LookupEnv("CI");ok {
77+
t.Skip("Detected CI, skipping reaper tests")
78+
}
79+
80+
errC:=make(chanerror,1)
81+
pids:=make(reap.PidCh,1)
82+
83+
// Use signals to notify when the child process is ready for the
84+
// next step of our test.
85+
usrSig:=make(chan os.Signal,1)
86+
signal.Notify(usrSig,syscall.SIGUSR1,syscall.SIGUSR2)
87+
defersignal.Stop(usrSig)
88+
89+
gofunc() {
90+
errC<-reaper.ForkReap(
91+
reaper.WithPIDCallback(pids),
92+
reaper.WithCatchSignals(os.Interrupt),
93+
// Signal propagation does not extend to children of children, so
94+
// we create a little bash script to ensure sleep is interrupted.
95+
reaper.WithExecArgs("/bin/sh","-c",fmt.Sprintf("pid=0; trap 'kill -USR2 %d; kill -TERM $pid' INT; sleep 10 &\npid=$!; kill -USR1 %d; wait",os.Getpid(),os.Getpid())),
96+
)
97+
}()
98+
99+
require.Equal(t,<-usrSig,syscall.SIGUSR1)
100+
err:=syscall.Kill(os.Getpid(),syscall.SIGINT)
101+
require.NoError(t,err)
102+
require.Equal(t,<-usrSig,syscall.SIGUSR2)
103+
104+
require.NoError(t,<-errC)
105+
}

‎agent/reaper/reaper_unix.go

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ package reaper
44

55
import (
66
"os"
7+
"os/signal"
78
"syscall"
89

910
"github.com/hashicorp/go-reap"
@@ -15,6 +16,24 @@ func IsInitProcess() bool {
1516
returnos.Getpid()==1
1617
}
1718

19+
funccatchSignals(pidint,sigs []os.Signal) {
20+
iflen(sigs)==0 {
21+
return
22+
}
23+
24+
sc:=make(chan os.Signal,1)
25+
signal.Notify(sc,sigs...)
26+
defersignal.Stop(sc)
27+
28+
for {
29+
s:=<-sc
30+
sig,ok:=s.(syscall.Signal)
31+
ifok {
32+
_=syscall.Kill(pid,sig)
33+
}
34+
}
35+
}
36+
1837
// ForkReap spawns a goroutine that reaps children. In order to avoid
1938
// complications with spawning `exec.Commands` in the same process that
2039
// is reaping, we forkexec a child process. This prevents a race between
@@ -51,13 +70,17 @@ func ForkReap(opt ...Option) error {
5170
}
5271

5372
//#nosec G204
54-
pid,_:=syscall.ForkExec(opts.ExecArgs[0],opts.ExecArgs,pattrs)
73+
pid,err:=syscall.ForkExec(opts.ExecArgs[0],opts.ExecArgs,pattrs)
74+
iferr!=nil {
75+
returnxerrors.Errorf("fork exec: %w",err)
76+
}
77+
78+
gocatchSignals(pid,opts.CatchSignals)
5579

5680
varwstatus syscall.WaitStatus
5781
_,err=syscall.Wait4(pid,&wstatus,0,nil)
5882
forxerrors.Is(err,syscall.EINTR) {
5983
_,err=syscall.Wait4(pid,&wstatus,0,nil)
6084
}
61-
62-
returnnil
85+
returnerr
6386
}

‎cli/agent.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,10 @@ func workspaceAgent() *cobra.Command {
6868
// Do not start a reaper on the child process. It's important
6969
// to do this else we fork bomb ourselves.
7070
args:=append(os.Args,"--no-reap")
71-
err:=reaper.ForkReap(reaper.WithExecArgs(args...))
71+
err:=reaper.ForkReap(
72+
reaper.WithExecArgs(args...),
73+
reaper.WithCatchSignals(InterruptSignals...),
74+
)
7275
iferr!=nil {
7376
logger.Error(ctx,"failed to reap",slog.Error(err))
7477
returnxerrors.Errorf("fork reap: %w",err)

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp