From 60876717b402f0dd6b4f585827779a9e435400c8 Mon Sep 17 00:00:00 2001
From: "Bryan C. Mills" <bcmills@google.com>
Date: Thu, 15 Jun 2023 17:41:37 -0400
Subject: [PATCH] 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 <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>
---
 src/cmd/go/go_unix_test.go       | 86 +++++++++++++++++++++++++++++++-
 src/cmd/go/internal/test/test.go | 10 +++-
 2 files changed, 94 insertions(+), 2 deletions(-)

diff --git a/src/cmd/go/go_unix_test.go b/src/cmd/go/go_unix_test.go
index bab9494401..d04e496778 100644
--- a/src/cmd/go/go_unix_test.go
+++ b/src/cmd/go/go_unix_test.go
@@ -2,12 +2,19 @@
 // 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"
+	"slices"
+	"strings"
 	"syscall"
 	"testing"
 )
@@ -33,3 +40,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(slices.Clip(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 2ce4c1a28e..995da15c90 100644
--- a/src/cmd/go/internal/test/test.go
+++ b/src/cmd/go/internal/test/test.go
@@ -1217,7 +1217,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.
-- 
2.50.0