diff --git a/cmd/groundstation/display.go b/cmd/groundstation/display.go index 13708e2..1c68117 100644 --- a/cmd/groundstation/display.go +++ b/cmd/groundstation/display.go @@ -82,7 +82,10 @@ func (s *displaySupervisor) run(ctx context.Context) { // stopDisplayProcess shuts the kiosk down deterministically: the interrupt // goes to the whole kiosk process group (runuser plus the kiosk itself, // which runuser does not forward signals to), waits for the grace window, -// then kills the group and reaps the direct child. +// then kills the group and reaps the direct child. The direct child exiting +// does not prove the group is empty — a kiosk that ignored the interrupt +// would outlive an exiting runuser — so the group receives a final SIGKILL +// sweep; for an already-empty group that is a harmless no-op. func stopDisplayProcess(cmd *exec.Cmd, done <-chan error, grace time.Duration) { signalGroup := func(sig syscall.Signal) { _ = syscall.Kill(-cmd.Process.Pid, sig) @@ -94,6 +97,7 @@ func stopDisplayProcess(cmd *exec.Cmd, done <-chan error, grace time.Duration) { signalGroup(syscall.SIGKILL) <-done } + signalGroup(syscall.SIGKILL) } // startDisplayProcess builds the kiosk launch command. It normally runs as diff --git a/cmd/groundstation/display_test.go b/cmd/groundstation/display_test.go index 7f07ce4..01f3a33 100644 --- a/cmd/groundstation/display_test.go +++ b/cmd/groundstation/display_test.go @@ -9,6 +9,7 @@ import ( "log" "os" "os/exec" + "os/signal" "runtime" "strconv" "strings" @@ -273,9 +274,14 @@ func TestStopDisplayProcessEscalatesToKillWhenInterruptIgnored(t *testing.T) { if runtime.GOOS != "linux" { t.Skip("procfs-based group check requires Linux") } + // Ignored dispositions are inherited by the whole process tree, so the + // group ignores SIGINT from birth without a trap-setup race. + signal.Ignore(syscall.SIGINT) + defer signal.Reset(syscall.SIGINT) - // The whole group ignores SIGINT, forcing the SIGKILL escalation. - cmd := exec.Command("sh", "-c", "echo started; trap '' INT; while :; do :; done") + // The group leader ignores SIGINT and never exits, forcing the + // SIGKILL escalation. + cmd := exec.Command("sh", "-c", "echo started; while :; do :; done") cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} pipe, err := cmd.StdoutPipe() if err != nil { @@ -301,3 +307,35 @@ func TestStopDisplayProcessEscalatesToKillWhenInterruptIgnored(t *testing.T) { waitForEmptyGroup(t, cmd.Process.Pid) } + +func TestStopDisplayProcessKillsGroupSurvivingDirectChildExit(t *testing.T) { + if runtime.GOOS != "linux" { + t.Skip("procfs-based group check requires Linux") + } + // The tree ignores SIGINT via inheritance, so a group member can + // outlive the direct child exactly like a kiosk behind an exiting + // runuser; the stop must sweep it instead of trusting the child exit. + signal.Ignore(syscall.SIGINT) + defer signal.Reset(syscall.SIGINT) + + cmd := exec.Command("sh", "-c", "echo started; sleep 60 & exit 0") + cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} + pipe, err := cmd.StdoutPipe() + if err != nil { + t.Fatal(err) + } + if err := cmd.Start(); err != nil { + t.Fatal(err) + } + greeting := make([]byte, len("started\n")) + if _, err := io.ReadFull(pipe, greeting); err != nil { + t.Fatalf("read readiness signal: %v", err) + } + + done := make(chan error, 1) + go func() { done <- cmd.Wait() }() + + stopDisplayProcess(cmd, done, 200*time.Millisecond) + + waitForEmptyGroup(t, cmd.Process.Pid) +}