Skip to content

Commit

Permalink
runtime: delay exiting while panic is running deferred functions
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
ianlancetaylor committed Jun 5, 2017
1 parent 3c745d7 commit 2d86f49
Show file tree
Hide file tree
Showing 5 changed files with 87 additions and 4 deletions.
29 changes: 29 additions & 0 deletions src/runtime/crash_test.go
Expand Up @@ -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)
}
}
}
21 changes: 20 additions & 1 deletion src/runtime/panic.go
Expand Up @@ -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 {
Expand Down Expand Up @@ -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.
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
13 changes: 11 additions & 2 deletions src/runtime/proc.go
Expand Up @@ -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)
}

Expand Down
1 change: 0 additions & 1 deletion src/runtime/runtime2.go
Expand Up @@ -720,7 +720,6 @@ var (
allm *m
allp [_MaxGomaxprocs + 1]*p
gomaxprocs int32
panicking uint32
ncpu int32
forcegc forcegcstate
sched schedt
Expand Down
27 changes: 27 additions & 0 deletions 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()
}

0 comments on commit 2d86f49

Please sign in to comment.