diff --git a/src/runtime/crash_test.go b/src/runtime/crash_test.go index 5333b60646..34f30c9a37 100644 --- a/src/runtime/crash_test.go +++ b/src/runtime/crash_test.go @@ -55,6 +55,16 @@ func runTestProg(t *testing.T, binary, name string, env ...string) string { t.Fatal(err) } + return runBuiltTestProg(t, exe, name, env...) +} + +func runBuiltTestProg(t *testing.T, exe, name string, env ...string) string { + if *flagQuick { + t.Skip("-quick") + } + + testenv.MustHaveGoBuild(t) + cmd := testenv.CleanCmdEnv(exec.Command(exe, name)) cmd.Env = append(cmd.Env, env...) if testing.Short() { @@ -64,7 +74,7 @@ func runTestProg(t *testing.T, binary, name string, env ...string) string { cmd.Stdout = &b cmd.Stderr = &b if err := cmd.Start(); err != nil { - t.Fatalf("starting %s %s: %v", binary, name, err) + t.Fatalf("starting %s %s: %v", exe, name, err) } // If the process doesn't complete within 1 minute, @@ -92,7 +102,7 @@ func runTestProg(t *testing.T, binary, name string, env ...string) string { }() if err := cmd.Wait(); err != nil { - t.Logf("%s %s exit status: %v", binary, name, err) + t.Logf("%s %s exit status: %v", exe, name, err) } close(done) diff --git a/src/runtime/proc.go b/src/runtime/proc.go index b423026c0e..e5823dd804 100644 --- a/src/runtime/proc.go +++ b/src/runtime/proc.go @@ -1820,10 +1820,16 @@ func startTemplateThread() { if GOARCH == "wasm" { // no threads on wasm yet return } + + // Disable preemption to guarantee that the template thread will be + // created before a park once haveTemplateThread is set. + mp := acquirem() if !atomic.Cas(&newmHandoff.haveTemplateThread, 0, 1) { + releasem(mp) return } newm(templateThread, nil) + releasem(mp) } // templateThread is a thread in a known-good state that exists solely diff --git a/src/runtime/proc_test.go b/src/runtime/proc_test.go index 764a279fca..de4dec36ce 100644 --- a/src/runtime/proc_test.go +++ b/src/runtime/proc_test.go @@ -7,6 +7,7 @@ package runtime_test import ( "fmt" "internal/race" + "internal/testenv" "math" "net" "runtime" @@ -929,6 +930,29 @@ func TestLockOSThreadAvoidsStatePropagation(t *testing.T) { } } +func TestLockOSThreadTemplateThreadRace(t *testing.T) { + testenv.MustHaveGoRun(t) + + exe, err := buildTestProg(t, "testprog") + if err != nil { + t.Fatal(err) + } + + iterations := 100 + if testing.Short() { + // Reduce run time to ~100ms, with much lower probability of + // catching issues. + iterations = 5 + } + for i := 0; i < iterations; i++ { + want := "OK\n" + output := runBuiltTestProg(t, exe, "LockOSThreadTemplateThreadRace") + if output != want { + t.Fatalf("run %d: want %q, got %q", i, want, output) + } + } +} + // fakeSyscall emulates a system call. //go:nosplit func fakeSyscall(duration time.Duration) { diff --git a/src/runtime/testdata/testprog/lockosthread.go b/src/runtime/testdata/testprog/lockosthread.go index fd3123e647..098cc4dd72 100644 --- a/src/runtime/testdata/testprog/lockosthread.go +++ b/src/runtime/testdata/testprog/lockosthread.go @@ -7,6 +7,7 @@ package main import ( "os" "runtime" + "sync" "time" ) @@ -30,6 +31,7 @@ func init() { runtime.LockOSThread() }) register("LockOSThreadAvoidsStatePropagation", LockOSThreadAvoidsStatePropagation) + register("LockOSThreadTemplateThreadRace", LockOSThreadTemplateThreadRace) } func LockOSThreadMain() { @@ -195,3 +197,50 @@ func LockOSThreadAvoidsStatePropagation() { runtime.UnlockOSThread() println("OK") } + +func LockOSThreadTemplateThreadRace() { + // This test attempts to reproduce the race described in + // golang.org/issue/38931. To do so, we must have a stop-the-world + // (achieved via ReadMemStats) racing with two LockOSThread calls. + // + // While this test attempts to line up the timing, it is only expected + // to fail (and thus hang) around 2% of the time if the race is + // present. + + // Ensure enough Ps to actually run everything in parallel. Though on + // <4 core machines, we are still at the whim of the kernel scheduler. + runtime.GOMAXPROCS(4) + + go func() { + // Stop the world; race with LockOSThread below. + var m runtime.MemStats + for { + runtime.ReadMemStats(&m) + } + }() + + // Try to synchronize both LockOSThreads. + start := time.Now().Add(10*time.Millisecond) + + var wg sync.WaitGroup + wg.Add(2) + + for i := 0; i < 2; i++ { + go func() { + for time.Now().Before(start) { + } + + // Add work to the local runq to trigger early startm + // in handoffp. + go func(){}() + + runtime.LockOSThread() + runtime.Gosched() // add a preemption point. + wg.Done() + }() + } + + wg.Wait() + // If both LockOSThreads completed then we did not hit the race. + println("OK") +}