From 3b23d576ea72235a3fef8f157eb5ab76e65854a8 Mon Sep 17 00:00:00 2001 From: Damien Neil Date: Mon, 8 Sep 2025 15:59:32 -0700 Subject: [PATCH] http2: fix race condition when disabling goroutine debugging for one test Fixes golang/go#66519 Change-Id: I7aecf20db44caaaf49754d62db193b8c42f3c63a Reviewed-on: https://go-review.googlesource.com/c/net/+/701836 Auto-Submit: Damien Neil Reviewed-by: Dmitri Shuralyov LUCI-TryBot-Result: Go LUCI Reviewed-by: Dmitri Shuralyov --- http2/gotrack.go | 17 ++++++++++++++--- http2/server_test.go | 7 ++++--- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/http2/gotrack.go b/http2/gotrack.go index 9933c9f8..9921ca09 100644 --- a/http2/gotrack.go +++ b/http2/gotrack.go @@ -15,21 +15,32 @@ import ( "runtime" "strconv" "sync" + "sync/atomic" ) var DebugGoroutines = os.Getenv("DEBUG_HTTP2_GOROUTINES") == "1" +// Setting DebugGoroutines to false during a test to disable goroutine debugging +// results in race detector complaints when a test leaves goroutines running before +// returning. Tests shouldn't do this, of course, but when they do it generally shows +// up as infrequent, hard-to-debug flakes. (See #66519.) +// +// Disable goroutine debugging during individual tests with an atomic bool. +// (Note that it's safe to enable/disable debugging mid-test, so the actual race condition +// here is harmless.) +var disableDebugGoroutines atomic.Bool + type goroutineLock uint64 func newGoroutineLock() goroutineLock { - if !DebugGoroutines { + if !DebugGoroutines || disableDebugGoroutines.Load() { return 0 } return goroutineLock(curGoroutineID()) } func (g goroutineLock) check() { - if !DebugGoroutines { + if !DebugGoroutines || disableDebugGoroutines.Load() { return } if curGoroutineID() != uint64(g) { @@ -38,7 +49,7 @@ func (g goroutineLock) check() { } func (g goroutineLock) checkNotOn() { - if !DebugGoroutines { + if !DebugGoroutines || disableDebugGoroutines.Load() { return } if curGoroutineID() == uint64(g) { diff --git a/http2/server_test.go b/http2/server_test.go index af1ebe04..71287d1e 100644 --- a/http2/server_test.go +++ b/http2/server_test.go @@ -3520,9 +3520,10 @@ func testServerContentLengthCanBeDisabled(t testing.TB) { } func disableGoroutineTracking(t testing.TB) { - old := DebugGoroutines - DebugGoroutines = false - t.Cleanup(func() { DebugGoroutines = old }) + disableDebugGoroutines.Store(true) + t.Cleanup(func() { + disableDebugGoroutines.Store(false) + }) } func BenchmarkServer_GetRequest(b *testing.B) {