internal/trace: only test for sync preemption if async preemption is off

Currently, the test change made for the fix to #68090 is flaky. This is
because the sync-point-only goroutine that we expect to be sync
preempted might only ever get async preempted in some circumstances.

This change adds a variant to all trace tests to run with
asyncpreemptoff=1, and the stacks test, the flaky one, only actually
checks for the sync-point in the trace when async preemption is
disabled.

Fixes #74417.

Change-Id: Ib6341bbc26921574b8f0fff6dd521ce83f85499c
Reviewed-on: https://go-review.googlesource.com/c/go/+/686055
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
This commit is contained in:
Michael Anthony Knyszek 2025-07-07 17:19:17 +00:00 committed by Michael Knyszek
parent ef46e1b164
commit 86b5ba7310

View file

@ -23,7 +23,7 @@ import (
) )
func TestTraceAnnotations(t *testing.T) { func TestTraceAnnotations(t *testing.T) {
testTraceProg(t, "annotations.go", func(t *testing.T, tb, _ []byte, _ bool) { testTraceProg(t, "annotations.go", func(t *testing.T, tb, _ []byte, _ string) {
type evDesc struct { type evDesc struct {
kind trace.EventKind kind trace.EventKind
task trace.TaskID task trace.TaskID
@ -98,7 +98,7 @@ func TestTraceCgoCallback(t *testing.T) {
} }
func TestTraceCPUProfile(t *testing.T) { func TestTraceCPUProfile(t *testing.T) {
testTraceProg(t, "cpu-profile.go", func(t *testing.T, tb, stderr []byte, _ bool) { testTraceProg(t, "cpu-profile.go", func(t *testing.T, tb, stderr []byte, _ string) {
// Parse stderr which has a CPU profile summary, if everything went well. // Parse stderr which has a CPU profile summary, if everything went well.
// (If it didn't, we shouldn't even make it here.) // (If it didn't, we shouldn't even make it here.)
scanner := bufio.NewScanner(bytes.NewReader(stderr)) scanner := bufio.NewScanner(bytes.NewReader(stderr))
@ -211,7 +211,7 @@ func TestTraceCPUProfile(t *testing.T) {
} }
func TestTraceFutileWakeup(t *testing.T) { func TestTraceFutileWakeup(t *testing.T) {
testTraceProg(t, "futile-wakeup.go", func(t *testing.T, tb, _ []byte, _ bool) { testTraceProg(t, "futile-wakeup.go", func(t *testing.T, tb, _ []byte, _ string) {
// Check to make sure that no goroutine in the "special" trace region // Check to make sure that no goroutine in the "special" trace region
// ends up blocking, unblocking, then immediately blocking again. // ends up blocking, unblocking, then immediately blocking again.
// //
@ -312,7 +312,7 @@ func TestTraceGOMAXPROCS(t *testing.T) {
} }
func TestTraceStacks(t *testing.T) { func TestTraceStacks(t *testing.T) {
testTraceProg(t, "stacks.go", func(t *testing.T, tb, _ []byte, stress bool) { testTraceProg(t, "stacks.go", func(t *testing.T, tb, _ []byte, godebug string) {
type frame struct { type frame struct {
fn string fn string
line int line int
@ -403,11 +403,19 @@ func TestTraceStacks(t *testing.T) {
{"runtime.GOMAXPROCS", 0}, {"runtime.GOMAXPROCS", 0},
{"main.main", 0}, {"main.main", 0},
}}, }},
{trace.EventStateTransition, "Goroutine Running->Runnable", []frame{ }
asyncPreemptOff := strings.Contains(godebug, "asyncpreemptoff=1")
if asyncPreemptOff {
// Only check for syncPreemptPoint if asynchronous preemption is disabled.
// Otherwise, the syncPreemptPoint goroutine might be exclusively async
// preempted, causing this test to flake.
syncPreemptEv := evDesc{trace.EventStateTransition, "Goroutine Running->Runnable", []frame{
{"main.syncPreemptPoint", 0}, {"main.syncPreemptPoint", 0},
{"main.main.func12", 0}, {"main.main.func12", 0},
}}, }}
want = append(want, syncPreemptEv)
} }
stress := strings.Contains(godebug, "traceadvanceperiod=0")
if !stress { if !stress {
// Only check for this stack if !stress because traceAdvance alone could // Only check for this stack if !stress because traceAdvance alone could
// allocate enough memory to trigger a GC if called frequently enough. // allocate enough memory to trigger a GC if called frequently enough.
@ -535,7 +543,7 @@ func TestTraceIterPull(t *testing.T) {
testTraceProg(t, "iter-pull.go", nil) testTraceProg(t, "iter-pull.go", nil)
} }
func checkReaderDeterminism(t *testing.T, tb, _ []byte, _ bool) { func checkReaderDeterminism(t *testing.T, tb, _ []byte, _ string) {
events := func() []trace.Event { events := func() []trace.Event {
var evs []trace.Event var evs []trace.Event
@ -572,7 +580,7 @@ func checkReaderDeterminism(t *testing.T, tb, _ []byte, _ bool) {
} }
} }
func testTraceProg(t *testing.T, progName string, extra func(t *testing.T, trace, stderr []byte, stress bool)) { func testTraceProg(t *testing.T, progName string, extra func(t *testing.T, trace, stderr []byte, godebug string)) {
testenv.MustHaveGoRun(t) testenv.MustHaveGoRun(t)
// Check if we're on a builder. // Check if we're on a builder.
@ -657,7 +665,7 @@ func testTraceProg(t *testing.T, progName string, extra func(t *testing.T, trace
// Run some extra validation. // Run some extra validation.
if !t.Failed() && extra != nil { if !t.Failed() && extra != nil {
extra(t, tb, errBuf.Bytes(), stress) extra(t, tb, errBuf.Bytes(), godebug)
} }
// Dump some more information on failure. // Dump some more information on failure.
@ -686,6 +694,9 @@ func testTraceProg(t *testing.T, progName string, extra func(t *testing.T, trace
t.Run("Default", func(t *testing.T) { t.Run("Default", func(t *testing.T) {
runTest(t, false, "") runTest(t, false, "")
}) })
t.Run("AsyncPreemptOff", func(t *testing.T) {
runTest(t, false, "asyncpreemptoff=1")
})
t.Run("Stress", func(t *testing.T) { t.Run("Stress", func(t *testing.T) {
if testing.Short() { if testing.Short() {
t.Skip("skipping trace stress tests in short mode") t.Skip("skipping trace stress tests in short mode")