Fix a race detected only if a test times out (#3808)

* Fix a race detected only if a test times out

I believe this can happen because the goroutine is started before the
command is started. Start the process first, then on success, start the
signal handler. Add a test for regression.

I verified the test fails without my change to wrap.go and the race is
from the signal handling goroutine.

* Register signal handler before cmd.Start()

go/tools/bzltestutil/wrap.go

@@ -124,23 +124,28 @@ func Wrap(pkg string) error {
 	if !filepath.IsAbs(exePath) && strings.ContainsRune(exePath, filepath.Separator) && chdir.TestExecDir != "" {
 		exePath = filepath.Join(chdir.TestExecDir, exePath)
 	}
+
+	c := make(chan os.Signal, 1)
+	signal.Notify(c, syscall.SIGTERM)
+
 	cmd := exec.Command(exePath, args...)
 	cmd.Env = append(os.Environ(), "GO_TEST_WRAP=0")
 	cmd.Stderr = io.MultiWriter(os.Stderr, streamMerger.ErrW)
 	cmd.Stdout = io.MultiWriter(os.Stdout, streamMerger.OutW)
-	go func() {
-		// When the Bazel test timeout is reached, Bazel sends a SIGTERM that
-		// we need to forward to the inner process.
-		// TODO: This never triggers on Windows, even though Go should simulate
-		//  a SIGTERM when Windows asks the process to close. It is not clear
-		//  whether Bazel uses the required graceful shutdown mechanism.
-		c := make(chan os.Signal, 1)
-		signal.Notify(c, syscall.SIGTERM)
-		<-c
-		cmd.Process.Signal(syscall.SIGTERM)
-	}()
 	streamMerger.Start()
-	err := cmd.Run()
+	err := cmd.Start()
+	if err == nil {
+		go func() {
+			// When the Bazel test timeout is reached, Bazel sends a SIGTERM that
+			// we need to forward to the inner process.
+			// TODO: This never triggers on Windows, even though Go should simulate
+			//  a SIGTERM when Windows asks the process to close. It is not clear
+			//  whether Bazel uses the required graceful shutdown mechanism.
+			<-c
+			cmd.Process.Signal(syscall.SIGTERM)
+		}()
+		err = cmd.Wait()
+	}
 	streamMerger.ErrW.Close()
 	streamMerger.OutW.Close()
 	streamMerger.Wait()

tests/core/race/race_test.go

@@ -94,6 +94,12 @@ go_test(
 		embed = [":coverrace"],
     race = "on",
 )
+
+go_test(
+    name = "timeout_test",
+    srcs = ["timeout_test.go"],
+    race = "on",
+)
 -- race_off.go --
 // +build !race
 
@@ -193,10 +199,41 @@ func TestCoverRace(t *testing.T) {
 		t.Errorf("got %d, want %d", got, 100)
 	}
 }
+
+-- timeout_test.go --
+package main
+
+import (
+	"testing"
+	"time"
+)
+
+func TestTimeout(t *testing.T) {
+	time.Sleep(10*time.Second)
+}
 `,
 	})
 }
 
+func TestTimeout(t *testing.T) {
+	cmd := bazel_testing.BazelCmd("test", "//:timeout_test", "--test_timeout=1", "--test_output=all")
+	stdout := &bytes.Buffer{}
+	cmd.Stdout = stdout
+	t.Logf("running: %s", strings.Join(cmd.Args, " "))
+	err := cmd.Run()
+	t.Log(stdout.String())
+	if err == nil {
+		t.Fatalf("expected bazel test to fail")
+	}
+	var xerr *exec.ExitError
+	if !errors.As(err, &xerr) || xerr.ExitCode() != bazel_testing.TESTS_FAILED {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if bytes.Contains(stdout.Bytes(), []byte("WARNING: DATA RACE")) {
+		t.Fatalf("unexpected data race; command failed with: %v\nstdout:\n%s", err, stdout.Bytes())
+	}
+}
+
 func Test(t *testing.T) {
 	for _, test := range []struct {
 		desc, cmd, target                    string