Skip to content

Commit 86b5ba7

Browse files
committed
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 <[email protected]> Reviewed-by: Cherry Mui <[email protected]>
1 parent ef46e1b commit 86b5ba7

File tree

1 file changed

+20
-9
lines changed

1 file changed

+20
-9
lines changed

src/internal/trace/trace_test.go

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ import (
2323
)
2424

2525
func TestTraceAnnotations(t *testing.T) {
26-
testTraceProg(t, "annotations.go", func(t *testing.T, tb, _ []byte, _ bool) {
26+
testTraceProg(t, "annotations.go", func(t *testing.T, tb, _ []byte, _ string) {
2727
type evDesc struct {
2828
kind trace.EventKind
2929
task trace.TaskID
@@ -98,7 +98,7 @@ func TestTraceCgoCallback(t *testing.T) {
9898
}
9999

100100
func TestTraceCPUProfile(t *testing.T) {
101-
testTraceProg(t, "cpu-profile.go", func(t *testing.T, tb, stderr []byte, _ bool) {
101+
testTraceProg(t, "cpu-profile.go", func(t *testing.T, tb, stderr []byte, _ string) {
102102
// Parse stderr which has a CPU profile summary, if everything went well.
103103
// (If it didn't, we shouldn't even make it here.)
104104
scanner := bufio.NewScanner(bytes.NewReader(stderr))
@@ -211,7 +211,7 @@ func TestTraceCPUProfile(t *testing.T) {
211211
}
212212

213213
func TestTraceFutileWakeup(t *testing.T) {
214-
testTraceProg(t, "futile-wakeup.go", func(t *testing.T, tb, _ []byte, _ bool) {
214+
testTraceProg(t, "futile-wakeup.go", func(t *testing.T, tb, _ []byte, _ string) {
215215
// Check to make sure that no goroutine in the "special" trace region
216216
// ends up blocking, unblocking, then immediately blocking again.
217217
//
@@ -312,7 +312,7 @@ func TestTraceGOMAXPROCS(t *testing.T) {
312312
}
313313

314314
func TestTraceStacks(t *testing.T) {
315-
testTraceProg(t, "stacks.go", func(t *testing.T, tb, _ []byte, stress bool) {
315+
testTraceProg(t, "stacks.go", func(t *testing.T, tb, _ []byte, godebug string) {
316316
type frame struct {
317317
fn string
318318
line int
@@ -403,11 +403,19 @@ func TestTraceStacks(t *testing.T) {
403403
{"runtime.GOMAXPROCS", 0},
404404
{"main.main", 0},
405405
}},
406-
{trace.EventStateTransition, "Goroutine Running->Runnable", []frame{
406+
}
407+
asyncPreemptOff := strings.Contains(godebug, "asyncpreemptoff=1")
408+
if asyncPreemptOff {
409+
// Only check for syncPreemptPoint if asynchronous preemption is disabled.
410+
// Otherwise, the syncPreemptPoint goroutine might be exclusively async
411+
// preempted, causing this test to flake.
412+
syncPreemptEv := evDesc{trace.EventStateTransition, "Goroutine Running->Runnable", []frame{
407413
{"main.syncPreemptPoint", 0},
408414
{"main.main.func12", 0},
409-
}},
415+
}}
416+
want = append(want, syncPreemptEv)
410417
}
418+
stress := strings.Contains(godebug, "traceadvanceperiod=0")
411419
if !stress {
412420
// Only check for this stack if !stress because traceAdvance alone could
413421
// allocate enough memory to trigger a GC if called frequently enough.
@@ -535,7 +543,7 @@ func TestTraceIterPull(t *testing.T) {
535543
testTraceProg(t, "iter-pull.go", nil)
536544
}
537545

538-
func checkReaderDeterminism(t *testing.T, tb, _ []byte, _ bool) {
546+
func checkReaderDeterminism(t *testing.T, tb, _ []byte, _ string) {
539547
events := func() []trace.Event {
540548
var evs []trace.Event
541549

@@ -572,7 +580,7 @@ func checkReaderDeterminism(t *testing.T, tb, _ []byte, _ bool) {
572580
}
573581
}
574582

575-
func testTraceProg(t *testing.T, progName string, extra func(t *testing.T, trace, stderr []byte, stress bool)) {
583+
func testTraceProg(t *testing.T, progName string, extra func(t *testing.T, trace, stderr []byte, godebug string)) {
576584
testenv.MustHaveGoRun(t)
577585

578586
// Check if we're on a builder.
@@ -657,7 +665,7 @@ func testTraceProg(t *testing.T, progName string, extra func(t *testing.T, trace
657665

658666
// Run some extra validation.
659667
if !t.Failed() && extra != nil {
660-
extra(t, tb, errBuf.Bytes(), stress)
668+
extra(t, tb, errBuf.Bytes(), godebug)
661669
}
662670

663671
// Dump some more information on failure.
@@ -686,6 +694,9 @@ func testTraceProg(t *testing.T, progName string, extra func(t *testing.T, trace
686694
t.Run("Default", func(t *testing.T) {
687695
runTest(t, false, "")
688696
})
697+
t.Run("AsyncPreemptOff", func(t *testing.T) {
698+
runTest(t, false, "asyncpreemptoff=1")
699+
})
689700
t.Run("Stress", func(t *testing.T) {
690701
if testing.Short() {
691702
t.Skip("skipping trace stress tests in short mode")

0 commit comments

Comments
 (0)