Skip to content

Commit 6087671

Browse files
Bryan C. Millsgopherbot
Bryan C. Mills
authored andcommitted
cmd/go/internal/test: don't wait for previous test actions when interrupted
Fixes #60203. Change-Id: I59a3320ede1eb3cf4443d7ea37b8cb39d01f222a Reviewed-on: https://go-review.googlesource.com/c/go/+/503936 Auto-Submit: Bryan Mills <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Run-TryBot: Bryan Mills <[email protected]> Reviewed-by: Ian Lance Taylor <[email protected]>
1 parent c1bc446 commit 6087671

File tree

2 files changed

+94
-2
lines changed

2 files changed

+94
-2
lines changed

src/cmd/go/go_unix_test.go

Lines changed: 85 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,19 @@
22
// Use of this source code is governed by a BSD-style
33
// license that can be found in the LICENSE file.
44

5-
//go:build darwin || dragonfly || freebsd || linux || netbsd || openbsd || solaris
5+
//go:build unix
66

77
package main_test
88

99
import (
10+
"bufio"
11+
"context"
12+
"internal/testenv"
13+
"io"
1014
"os"
15+
"os/exec"
16+
"slices"
17+
"strings"
1118
"syscall"
1219
"testing"
1320
)
@@ -33,3 +40,80 @@ func TestGoBuildUmask(t *testing.T) {
3340
t.Fatalf("wrote x with mode=%v, wanted no 0077 bits", mode)
3441
}
3542
}
43+
44+
// TestTestInterrupt verifies the fix for issue #60203.
45+
//
46+
// If the whole process group for a 'go test' invocation receives
47+
// SIGINT (as would be sent by pressing ^C on a console),
48+
// it should return quickly, not deadlock.
49+
func TestTestInterrupt(t *testing.T) {
50+
if testing.Short() {
51+
t.Skipf("skipping in short mode: test executes many subprocesses")
52+
}
53+
// Don't run this test in parallel, for the same reason.
54+
55+
tg := testgo(t)
56+
defer tg.cleanup()
57+
tg.setenv("GOROOT", testGOROOT)
58+
59+
ctx, cancel := context.WithCancel(context.Background())
60+
cmd := testenv.CommandContext(t, ctx, tg.goTool(), "test", "std", "-short", "-count=1")
61+
cmd.Dir = tg.execDir
62+
63+
// Override $TMPDIR when running the tests: since we're terminating the tests
64+
// with a signal they might fail to clean up some temp files, and we don't
65+
// want that to cause an "unexpected files" failure at the end of the run.
66+
cmd.Env = append(slices.Clip(tg.env), tempEnvName()+"="+t.TempDir())
67+
68+
cmd.SysProcAttr = &syscall.SysProcAttr{
69+
Setpgid: true,
70+
}
71+
cmd.Cancel = func() error {
72+
pgid := cmd.Process.Pid
73+
return syscall.Kill(-pgid, syscall.SIGINT)
74+
}
75+
76+
pipe, err := cmd.StdoutPipe()
77+
if err != nil {
78+
t.Fatal(err)
79+
}
80+
81+
t.Logf("running %v", cmd)
82+
if err := cmd.Start(); err != nil {
83+
t.Fatal(err)
84+
}
85+
86+
stdout := new(strings.Builder)
87+
r := bufio.NewReader(pipe)
88+
line, err := r.ReadString('\n')
89+
if err != nil {
90+
t.Fatal(err)
91+
}
92+
stdout.WriteString(line)
93+
94+
// The output line for some test was written, so we know things are in progress.
95+
//
96+
// Cancel the rest of the run by sending SIGINT to the process group:
97+
// it should finish up and exit with a nonzero status,
98+
// not have to be killed with SIGKILL.
99+
cancel()
100+
101+
io.Copy(stdout, r)
102+
if stdout.Len() > 0 {
103+
t.Logf("stdout:\n%s", stdout)
104+
}
105+
err = cmd.Wait()
106+
107+
ee, _ := err.(*exec.ExitError)
108+
if ee == nil {
109+
t.Fatalf("unexpectedly finished with nonzero status")
110+
}
111+
if len(ee.Stderr) > 0 {
112+
t.Logf("stderr:\n%s", ee.Stderr)
113+
}
114+
if !ee.Exited() {
115+
t.Fatalf("'go test' did not exit after interrupt: %v", err)
116+
}
117+
118+
t.Logf("interrupted tests without deadlocking")
119+
}

src/cmd/go/internal/test/test.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1217,7 +1217,15 @@ func (lockedStdout) Write(b []byte) (int, error) {
12171217

12181218
func (r *runTestActor) Act(b *work.Builder, ctx context.Context, a *work.Action) error {
12191219
// Wait for previous test to get started and print its first json line.
1220-
<-r.prev
1220+
select {
1221+
case <-r.prev:
1222+
case <-base.Interrupted:
1223+
// We can't wait for the previous test action to complete: we don't start
1224+
// new actions after an interrupt, so if that action wasn't already running
1225+
// it might never happen. Instead, just don't log anything for this action.
1226+
base.SetExitStatus(1)
1227+
return nil
1228+
}
12211229

12221230
if a.Failed {
12231231
// We were unable to build the binary.

0 commit comments

Comments
 (0)