gopls/internal/test/integration/web: kill "go tool trace" processes

Clearly our cleanup mechanism isn't good enough since it causes
"go tool trace" processes to accumulate on builder machines,
causing OOM on ppc. Explicitly kill traceviewers once the test
finishes.

This is a global hammer that won't play nicely with multiple
concurrent tests of FlightRecorder. (Currently there is only
a single test.)

Fixes golang/go#74668
Updates golang/go#73988

Change-Id: Iea823619d188d0d4ae1157d27c90fc6a2fc34617
Reviewed-on: https://go-review.googlesource.com/c/tools/+/689195
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Michael Pratt <[email protected]>

Author: adonovan

gopls/internal/debug/flight.go

@@ -20,6 +20,22 @@ import (
 	"time"
 )
 
+var (
+	traceviewersMu sync.Mutex
+	traceviewers   []*os.Process
+)
+
+// KillTraceViewers kills all "go tool trace" processes started by
+// /flightrecorder requests, for use in tests (see #74668).
+func KillTraceViewers() {
+	traceviewersMu.Lock()
+	for _, p := range traceviewers {
+		p.Kill() // ignore error
+	}
+	traceviewers = nil
+	traceviewersMu.Unlock()
+}
+
 // The FlightRecorder is a global resource, so create at most one per process.
 var getRecorder = sync.OnceValues(func() (*trace.FlightRecorder, error) {
 	fr := trace.NewFlightRecorder(trace.FlightRecorderConfig{
@@ -53,7 +69,7 @@ func startFlightRecorder() (http.HandlerFunc, error) {
 			return
 		}
 		if _, err := fr.WriteTo(f); err != nil {
-			f.Close()
+			f.Close() // ignore error
 			errorf("failed to write flight record: %s", err)
 			return
 		}
@@ -113,6 +129,11 @@ func startFlightRecorder() (http.HandlerFunc, error) {
 			return
 		}
 
+		// Save the process so we can kill it when tests finish.
+		traceviewersMu.Lock()
+		traceviewers = append(traceviewers, cmd.Process)
+		traceviewersMu.Unlock()
+
 		// Some of the CI builders can be quite heavily loaded.
 		// Give them an extra grace period.
 		timeout := 10 * time.Second

gopls/internal/debug/flight_go124.go

@@ -14,3 +14,5 @@ import (
 func startFlightRecorder() (http.HandlerFunc, error) {
 	return nil, errors.ErrUnsupported
 }
+
+func KillTraceViewers() {}

gopls/internal/test/integration/web/flight_test.go

@@ -9,6 +9,7 @@ import (
 	"runtime"
 	"testing"
 
+	"golang.org/x/tools/gopls/internal/debug"
 	"golang.org/x/tools/gopls/internal/protocol"
 	"golang.org/x/tools/gopls/internal/protocol/command"
 	. "golang.org/x/tools/gopls/internal/test/integration"
@@ -28,6 +29,10 @@ func TestFlightRecorder(t *testing.T) {
 	}
 	testenv.NeedsGo1Point(t, 25)
 
+	// This is a global hammer; it won't play nicely with
+	// multiple concurrent tests of Flight Recorder.
+	t.Cleanup(debug.KillTraceViewers)
+
 	const files = `
 -- go.mod --
 module example.com