net/http/cgi: in TestCopyError, check for a Handler.ServeHTTP goroutine instead of a running PID

Previously, the test could fail spuriously if the CGI process's PID
happened to be reused in between checks. That sort of reuse is highly
unlikely on platforms that cycle through the PID space sequentially
(such as Linux), but plausible on platforms that use randomized PIDs
(such as OpenBSD).

Also unskip the test on Windows, since it no longer relies on being
able to send signal 0 to an arbitrary PID.

Also change the expected failure mode of the test to a timeout instead
of a call to t.Fatalf, so that on failure we get a useful goroutine
dump for debugging instead of a non-actionable failure message.

Fixes #57369 (maybe).

Change-Id: Ib7e3fff556450b48cb5e6ea120fdf4d53547479b
Reviewed-on: https://go-review.googlesource.com/c/go/+/554075
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
Reviewed-by: Damien Neil <dneil@google.com>

src/net/http/cgi/host_test.go

@@ -17,8 +17,8 @@ import (
 	"os"
 	"path/filepath"
 	"reflect"
+	"regexp"
 	"runtime"
-	"strconv"
 	"strings"
 	"testing"
 	"time"
@@ -363,11 +363,12 @@ func TestInternalRedirect(t *testing.T) {
 
 // TestCopyError tests that we kill the process if there's an error copying
 // its output. (for example, from the client having gone away)
+//
+// If we fail to do so, the test will time out (and dump its goroutines) with a
+// call to [Handler.ServeHTTP] blocked on a deferred call to [exec.Cmd.Wait].
 func TestCopyError(t *testing.T) {
 	testenv.MustHaveExec(t)
-	if runtime.GOOS == "windows" {
+
-		t.Skipf("skipping test on %q", runtime.GOOS)
-	}
 	h := &Handler{
 		Path: os.Args[0],
 		Root: "/test.cgi",
@@ -390,37 +391,42 @@ func TestCopyError(t *testing.T) {
 		t.Fatalf("ReadResponse: %v", err)
 	}
 
-	pidstr := res.Header.Get("X-CGI-Pid")
-	if pidstr == "" {
-		t.Fatalf("expected an X-CGI-Pid header in response")
-	}
-	pid, err := strconv.Atoi(pidstr)
-	if err != nil {
-		t.Fatalf("invalid X-CGI-Pid value")
-	}
-
 	var buf [5000]byte
 	n, err := io.ReadFull(res.Body, buf[:])
 	if err != nil {
 		t.Fatalf("ReadFull: %d bytes, %v", n, err)
 	}
 
-	childRunning := func() bool {
+	if !handlerRunning() {
-		return isProcessRunning(pid)
+		t.Fatalf("pre-conn.Close, expected handler to still be running")
-	}
-
-	if !childRunning() {
-		t.Fatalf("pre-conn.Close, expected child to be running")
 	}
 	conn.Close()
+	closed := time.Now()
 
-	tries := 0
+	nextSleep := 1 * time.Millisecond
-	for tries < 25 && childRunning() {
+	for {
-		time.Sleep(50 * time.Millisecond * time.Duration(tries))
+		time.Sleep(nextSleep)
-		tries++
+		nextSleep *= 2
+		if !handlerRunning() {
+			break
+		}
+		t.Logf("handler still running %v after conn.Close", time.Since(closed))
 	}
-	if childRunning() {
+}
-		t.Fatalf("post-conn.Close, expected child to be gone")
+
+// handlerRunning reports whether any goroutine is currently running
+// [Handler.ServeHTTP].
+func handlerRunning() bool {
+	r := regexp.MustCompile(`net/http/cgi\.\(\*Handler\)\.ServeHTTP`)
+	buf := make([]byte, 64<<10)
+	for {
+		n := runtime.Stack(buf, true)
+		if n < len(buf) {
+			return r.Match(buf[:n])
+		}
+		// Buffer wasn't large enough for a full goroutine dump.
+		// Resize it and try again.
+		buf = make([]byte, 2*len(buf))
 	}
 }
 

src/net/http/cgi/plan9_test.go

@@ -1,17 +0,0 @@
-// Copyright 2013 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-//go:build plan9
-
-package cgi
-
-import (
-	"os"
-	"strconv"
-)
-
-func isProcessRunning(pid int) bool {
-	_, err := os.Stat("/proc/" + strconv.Itoa(pid))
-	return err == nil
-}

src/net/http/cgi/posix_test.go

@@ -1,20 +0,0 @@
-// Copyright 2013 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-//go:build !plan9
-
-package cgi
-
-import (
-	"os"
-	"syscall"
-)
-
-func isProcessRunning(pid int) bool {
-	p, err := os.FindProcess(pid)
-	if err != nil {
-		return false
-	}
-	return p.Signal(syscall.Signal(0)) == nil
-}