runtime: prioritize panic output over racefini

For some reason CL 646198 uncovered #3934 and #20018 again, but only in
race mode. It turns out that because racefini does not return, and
racefini is called early after main returns, we would not properly wait
for a concurrent panic to complete. This would result in fairly
consistent failures of TestPanicRace, which specifically looks for the
panic output to appear if main concurrently exits.

The important part of this change is that race mode will no longer have
the bug described in #3934 and #20018. A byproduct, however, is that
racefini is that we're essentially prioritizing the panic output over
racefini in this scenario. If racefini were to reveal a latent race
condition and fail, we'll prefer to surface the panic. Such a case is
probably fine, because the panic is always an crashing, unrecoverable
panic.

For #3934.
For #20018.

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

diff --git a/src/runtime/crash_test.go b/src/runtime/crash_test.go
index 7d02363cc994aa2ebebf3ca84c53ba61e069b0e2..2b8ca549ad84f214446af09ab1f518a3ff47bf9f 100644 (file)
--- a/src/runtime/crash_test.go
+++ b/src/runtime/crash_test.go
@@ -788,9 +788,6 @@ func TestPanicInlined(t *testing.T) {
 // Test for issues #3934 and #20018.
 // We want to delay exiting until a panic print is complete.
 func TestPanicRace(t *testing.T) {
-       if race.Enabled {
-               t.Skip("racefini exits program before main can delay exiting")
-       }
        testenv.MustHaveGoRun(t)
 
        exe, err := buildTestProg(t, "testprog")

diff --git a/src/runtime/proc.go b/src/runtime/proc.go
index 89080b48de861f03b3e14d93d066a9d7826e4026..ef3a0b7a0e4c60104b8067a91ecae05c7cefa488 100644 (file)
--- a/src/runtime/proc.go
+++ b/src/runtime/proc.go
@@ -288,13 +288,6 @@ func main() {
        fn := main_main // make an indirect call, as the linker doesn't know the address of the main package when laying down the runtime
        fn()
 
-       exitHooksRun := false
-       if raceenabled {
-               runExitHooks(0) // run hooks now, since racefini does not return
-               exitHooksRun = true
-               racefini()
-       }
-
        // Check for C memory leaks if using ASAN and we've made cgo calls,
        // or if we are running as a library in a C program.
        // We always make one cgo call, above, to notify_runtime_init_done,
@@ -302,6 +295,7 @@ func main() {
        // No point in leak checking if no cgo calls, since leak checking
        // just looks for objects allocated using malloc and friends.
        // Just checking iscgo doesn't help because asan implies iscgo.
+       exitHooksRun := false
        if asanenabled && (isarchive || islibrary || NumCgoCall() > 1) {
                runExitHooks(0) // lsandoleakcheck may not return
                exitHooksRun = true
@@ -327,6 +321,9 @@ func main() {
        if !exitHooksRun {
                runExitHooks(0)
        }
+       if raceenabled {
+               racefini() // does not return
+       }
 
        exit(0)
        for {