runtime: delay exiting while panic is running deferred functions

Try to avoid a race between the main goroutine exiting and a panic
occurring. Don't try too hard, to avoid hanging.

Updates #3934
Fixes #20018

Change-Id: I57a02b6d795d2a61f1cadd137ce097145280ece7
Reviewed-on: https://go-review.googlesource.com/41052
Reviewed-by: Austin Clements <austin@google.com>

diff --git a/src/runtime/crash_test.go b/src/runtime/crash_test.go
index f6a0cd6cbb54f68b0db7752351b89301eabb3c70..b08dd87d9b1e90f2500d5e7eebc1c02ee1f8d934 100644 (file)
--- a/src/runtime/crash_test.go
+++ b/src/runtime/crash_test.go
@@ -568,3 +568,32 @@ func TestPanicInlined(t *testing.T) {
        pt := new(point)
        pt.negate()
 }
+
+// Test for issues #3934 and #20018.
+// We want to delay exiting until a panic print is complete.
+func TestPanicRace(t *testing.T) {
+       testenv.MustHaveGoRun(t)
+
+       exe, err := buildTestProg(t, "testprog")
+       if err != nil {
+               t.Fatal(err)
+       }
+
+       got, err := testEnv(exec.Command(exe, "PanicRace")).CombinedOutput()
+       if err == nil {
+               t.Error("program exited successfully, should have failed")
+       }
+
+       t.Logf("%s\n", got)
+
+       wants := []string{
+               "panic: crash",
+               "PanicRace",
+               "created by ",
+       }
+       for _, want := range wants {
+               if !bytes.Contains(got, []byte(want)) {
+                       t.Errorf("did not find expected string %q", want)
+               }
+       }
+}

diff --git a/src/runtime/panic.go b/src/runtime/panic.go
index f099f2292c87fda1d56db2bc6d78c16b0deaed38..43bfdd7a1e15e6aca6c16ff8d2e2a3079fd8bded 100644 (file)
--- a/src/runtime/panic.go
+++ b/src/runtime/panic.go
@@ -456,6 +456,8 @@ func gopanic(e interface{}) {
        p.link = gp._panic
        gp._panic = (*_panic)(noescape(unsafe.Pointer(&p)))
 
+       atomic.Xadd(&runningPanicDefers, 1)
+
        for {
                d := gp._defer
                if d == nil {
@@ -504,6 +506,8 @@ func gopanic(e interface{}) {
                sp := unsafe.Pointer(d.sp) // must be pointer so it gets adjusted during stack copy
                freedefer(d)
                if p.recovered {
+                       atomic.Xadd(&runningPanicDefers, -1)
+
                        gp._panic = p.link
                        // Aborted panics are marked but remain on the g.panic list.
                        // Remove them from the list.
@@ -527,6 +531,11 @@ func gopanic(e interface{}) {
        // and String methods to prepare the panic strings before startpanic.
        preprintpanics(gp._panic)
        startpanic()
+
+       // startpanic set panicking, which will block main from exiting,
+       // so now OK to decrement runningPanicDefers.
+       atomic.Xadd(&runningPanicDefers, -1)
+
        printpanics(gp._panic)
        dopanic(0)       // should not return
        *(*int)(nil) = 0 // not reached
@@ -597,7 +606,17 @@ func throw(s string) {
        *(*int)(nil) = 0 // not reached
 }
 
-//uint32 runtime·panicking;
+// runningPanicDefers is non-zero while running deferred functions for panic.
+// runningPanicDefers is incremented and decremented atomically.
+// This is used to try hard to get a panic stack trace out when exiting.
+var runningPanicDefers uint32
+
+// panicking is non-zero when crashing the program for an unrecovered panic.
+// panicking is incremented and decremented atomically.
+var panicking uint32
+
+// paniclk is held while printing the panic information and stack trace,
+// so that two concurrent panics don't overlap their output.
 var paniclk mutex
 
 // Unwind the stack after a deferred function calls recover

diff --git a/src/runtime/proc.go b/src/runtime/proc.go
index 7d6b89016a6052451e938f2207be5438fb07d085..24a62492e1149b94e7a995703503951848bcd939 100644 (file)
--- a/src/runtime/proc.go
+++ b/src/runtime/proc.go
@@ -190,8 +190,17 @@ func main() {
        // Make racy client program work: if panicking on
        // another goroutine at the same time as main returns,
        // let the other goroutine finish printing the panic trace.
-       // Once it does, it will exit. See issue 3934.
-       if panicking != 0 {
+       // Once it does, it will exit. See issues 3934 and 20018.
+       if atomic.Load(&runningPanicDefers) != 0 {
+               // Running deferred functions should not take long.
+               for c := 0; c < 1000; c++ {
+                       if atomic.Load(&runningPanicDefers) == 0 {
+                               break
+                       }
+                       Gosched()
+               }
+       }
+       if atomic.Load(&panicking) != 0 {
                gopark(nil, nil, "panicwait", traceEvGoStop, 1)
        }
 

diff --git a/src/runtime/runtime2.go b/src/runtime/runtime2.go
index b0ebfd818c6a3c5747fd4f0d5763e613959413a8..da57235b0276f570891bd6a09c5acd6dada449d5 100644 (file)
--- a/src/runtime/runtime2.go
+++ b/src/runtime/runtime2.go
@@ -720,7 +720,6 @@ var (
        allm        *m
        allp        [_MaxGomaxprocs + 1]*p
        gomaxprocs  int32
-       panicking   uint32
        ncpu        int32
        forcegc     forcegcstate
        sched       schedt

diff --git a/src/runtime/testdata/testprog/panicrace.go b/src/runtime/testdata/testprog/panicrace.go
new file mode 100644 (file)
index 0000000..f058994
--- /dev/null
+++ b/src/runtime/testdata/testprog/panicrace.go
@@ -0,0 +1,27 @@
+// Copyright 2017 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package main
+
+import (
+       "runtime"
+       "sync"
+)
+
+func init() {
+       register("PanicRace", PanicRace)
+}
+
+func PanicRace() {
+       var wg sync.WaitGroup
+       wg.Add(1)
+       go func() {
+               defer func() {
+                       wg.Done()
+                       runtime.Gosched()
+               }()
+               panic("crash")
+       }()
+       wg.Wait()
+}