runtime: print fatal messages without interleaving

Grab the print lock around the set of prints we use to report
fatal errors. This ensures that each fatal error gets reported
atomically instead of interleaved with other fatal errors.

Fixes #69447

Change-Id: Ib3569f0c8210fd7e19a7d8ef4bc114f07469f317
Reviewed-on: https://go-review.googlesource.com/c/go/+/615655
Auto-Submit: Keith Randall <khr@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Keith Randall <khr@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

diff --git a/src/runtime/crash_test.go b/src/runtime/crash_test.go
index 69e1034ff825af54e3190d9485af8982dcb37ea8..52d33b8f58cf3e1c455d62c56ab864c2ba016fb8 100644 (file)
--- a/src/runtime/crash_test.go
+++ b/src/runtime/crash_test.go
@@ -621,7 +621,7 @@ func TestConcurrentMapWrites(t *testing.T) {
        }
        testenv.MustHaveGoRun(t)
        output := runTestProg(t, "testprog", "concurrentMapWrites")
-       want := "fatal error: concurrent map writes"
+       want := "fatal error: concurrent map writes\n"
        if !strings.HasPrefix(output, want) {
                t.Fatalf("output does not start with %q:\n%s", want, output)
        }
@@ -632,7 +632,7 @@ func TestConcurrentMapReadWrite(t *testing.T) {
        }
        testenv.MustHaveGoRun(t)
        output := runTestProg(t, "testprog", "concurrentMapReadWrite")
-       want := "fatal error: concurrent map read and map write"
+       want := "fatal error: concurrent map read and map write\n"
        if !strings.HasPrefix(output, want) {
                t.Fatalf("output does not start with %q:\n%s", want, output)
        }
@@ -643,12 +643,36 @@ func TestConcurrentMapIterateWrite(t *testing.T) {
        }
        testenv.MustHaveGoRun(t)
        output := runTestProg(t, "testprog", "concurrentMapIterateWrite")
-       want := "fatal error: concurrent map iteration and map write"
+       want := "fatal error: concurrent map iteration and map write\n"
        if !strings.HasPrefix(output, want) {
                t.Fatalf("output does not start with %q:\n%s", want, output)
        }
 }
 
+func TestConcurrentMapWritesIssue69447(t *testing.T) {
+       testenv.MustHaveGoRun(t)
+       exe, err := buildTestProg(t, "testprog")
+       if err != nil {
+               t.Fatal(err)
+       }
+       for i := 0; i < 200; i++ {
+               output := runBuiltTestProg(t, exe, "concurrentMapWrites")
+               if output == "" {
+                       // If we didn't detect an error, that's ok.
+                       // This case makes this test not flaky like
+                       // the other ones above.
+                       // (More correctly, this case makes this test flaky
+                       // in the other direction, in that it might not
+                       // detect a problem even if there is one.)
+                       continue
+               }
+               want := "fatal error: concurrent map writes\n"
+               if !strings.HasPrefix(output, want) {
+                       t.Fatalf("output does not start with %q:\n%s", want, output)
+               }
+       }
+}
+
 type point struct {
        x, y *int
 }

diff --git a/src/runtime/panic.go b/src/runtime/panic.go
index 5b62e019d92b5a13b0e857c8f5c90d1182b3b0bd..d70d5679120541838c275c93491bd6fa749bca57 100644 (file)
--- a/src/runtime/panic.go
+++ b/src/runtime/panic.go
@@ -1081,6 +1081,7 @@ func throw(s string) {
 func fatal(s string) {
        // Everything fatal does should be recursively nosplit so it
        // can be called even when it's unsafe to grow the stack.
+       printlock() // Prevent multiple interleaved fatal reports. See issue 69447.
        systemstack(func() {
                print("fatal error: ")
                printindented(s) // logically printpanicval(s), but avoids convTstring write barrier
@@ -1088,6 +1089,7 @@ func fatal(s string) {
        })
 
        fatalthrow(throwTypeUser)
+       printunlock()
 }
 
 // runningPanicDefers is non-zero while running deferred functions for panic.