[release-branch.go1.20] cmd/go/internal/test: don't wait for previous test actions...

Fixes #60849.
Updates #60203.

Change-Id: I59a3320ede1eb3cf4443d7ea37b8cb39d01f222a
Reviewed-on: https://go-review.googlesource.com/c/go/+/503936
Auto-Submit: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
(cherry picked from commit 60876717b402f0dd6b4f585827779a9e435400c8)
Reviewed-on: https://go-review.googlesource.com/c/go/+/504062
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>

diff --git a/src/cmd/go/go_unix_test.go b/src/cmd/go/go_unix_test.go
index bab94944014afa330431c328ca22edce9d80ba31..7cc68257f6ff3c29923175584bf3478e50908c6b 100644 (file)
--- a/src/cmd/go/go_unix_test.go
+++ b/src/cmd/go/go_unix_test.go
@@ -2,12 +2,18 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-//go:build darwin || dragonfly || freebsd || linux || netbsd || openbsd || solaris
+//go:build unix
 
 package main_test
 
 import (
+       "bufio"
+       "context"
+       "internal/testenv"
+       "io"
        "os"
+       "os/exec"
+       "strings"
        "syscall"
        "testing"
 )
@@ -33,3 +39,80 @@ func TestGoBuildUmask(t *testing.T) {
                t.Fatalf("wrote x with mode=%v, wanted no 0077 bits", mode)
        }
 }
+
+// TestTestInterrupt verifies the fix for issue #60203.
+//
+// If the whole process group for a 'go test' invocation receives
+// SIGINT (as would be sent by pressing ^C on a console),
+// it should return quickly, not deadlock.
+func TestTestInterrupt(t *testing.T) {
+       if testing.Short() {
+               t.Skipf("skipping in short mode: test executes many subprocesses")
+       }
+       // Don't run this test in parallel, for the same reason.
+
+       tg := testgo(t)
+       defer tg.cleanup()
+       tg.setenv("GOROOT", testGOROOT)
+
+       ctx, cancel := context.WithCancel(context.Background())
+       cmd := testenv.CommandContext(t, ctx, tg.goTool(), "test", "std", "-short", "-count=1")
+       cmd.Dir = tg.execDir
+
+       // Override $TMPDIR when running the tests: since we're terminating the tests
+       // with a signal they might fail to clean up some temp files, and we don't
+       // want that to cause an "unexpected files" failure at the end of the run.
+       cmd.Env = append(tg.env[:len(tg.env):len(tg.env)], tempEnvName()+"="+t.TempDir())
+
+       cmd.SysProcAttr = &syscall.SysProcAttr{
+               Setpgid: true,
+       }
+       cmd.Cancel = func() error {
+               pgid := cmd.Process.Pid
+               return syscall.Kill(-pgid, syscall.SIGINT)
+       }
+
+       pipe, err := cmd.StdoutPipe()
+       if err != nil {
+               t.Fatal(err)
+       }
+
+       t.Logf("running %v", cmd)
+       if err := cmd.Start(); err != nil {
+               t.Fatal(err)
+       }
+
+       stdout := new(strings.Builder)
+       r := bufio.NewReader(pipe)
+       line, err := r.ReadString('\n')
+       if err != nil {
+               t.Fatal(err)
+       }
+       stdout.WriteString(line)
+
+       // The output line for some test was written, so we know things are in progress.
+       //
+       // Cancel the rest of the run by sending SIGINT to the process group:
+       // it should finish up and exit with a nonzero status,
+       // not have to be killed with SIGKILL.
+       cancel()
+
+       io.Copy(stdout, r)
+       if stdout.Len() > 0 {
+               t.Logf("stdout:\n%s", stdout)
+       }
+       err = cmd.Wait()
+
+       ee, _ := err.(*exec.ExitError)
+       if ee == nil {
+               t.Fatalf("unexpectedly finished with nonzero status")
+       }
+       if len(ee.Stderr) > 0 {
+               t.Logf("stderr:\n%s", ee.Stderr)
+       }
+       if !ee.Exited() {
+               t.Fatalf("'go test' did not exit after interrupt: %v", err)
+       }
+
+       t.Logf("interrupted tests without deadlocking")
+}

diff --git a/src/cmd/go/internal/test/test.go b/src/cmd/go/internal/test/test.go
index 28bfabdcb6f9621e5ba0a5c2cd54406a62e7d70b..6815bbc8a10b5612c31fd5ba6e4ba9138fcce7a3 100644 (file)
--- a/src/cmd/go/internal/test/test.go
+++ b/src/cmd/go/internal/test/test.go
@@ -1148,7 +1148,15 @@ func (lockedStdout) Write(b []byte) (int, error) {
 
 func (r *runTestActor) Act(b *work.Builder, ctx context.Context, a *work.Action) error {
        // Wait for previous test to get started and print its first json line.
-       <-r.prev
+       select {
+       case <-r.prev:
+       case <-base.Interrupted:
+               // We can't wait for the previous test action to complete: we don't start
+               // new actions after an interrupt, so if that action wasn't already running
+               // it might never happen. Instead, just don't log anything for this action.
+               base.SetExitStatus(1)
+               return nil
+       }
 
        if a.Failed {
                // We were unable to build the binary.