From 7dcd343ed641d3b70c09153d3b041ca3fe83b25e Mon Sep 17 00:00:00 2001 From: Dan Scales Date: Wed, 9 Oct 2019 12:18:26 -0700 Subject: runtime: ensure that Goexit cannot be aborted by a recursive panic/recover When we do a successful recover of a panic, we resume normal execution by returning from the frame that had the deferred call that did the recover (after executing any remaining deferred calls in that frame). However, suppose we have called runtime.Goexit and there is a panic during one of the deferred calls run by the Goexit. Further assume that there is a deferred call in the frame of the Goexit or a parent frame that does a recover. Then the recovery process will actually resume normal execution above the Goexit frame and hence abort the Goexit. We will not terminate the thread as expected, but continue running in the frame above the Goexit. To fix this, we explicitly create a _panic object for a Goexit call. We then change the "abort" behavior for Goexits, but not panics. After a recovery, if the top-level panic is actually a Goexit that is marked to be aborted, then we return to the Goexit defer-processing loop, so that the Goexit is not actually aborted. Actual code changes are just panic.go, runtime2.go, and funcid.go. Adjusted the test related to the new Goexit behavior (TestRecoverBeforePanicAfterGoexit) and added several new tests of aborted panics (whose behavior has not changed). Fixes #29226 Change-Id: Ib13cb0074f5acc2567a28db7ca6912cfc47eecb5 Reviewed-on: https://go-review.googlesource.com/c/go/+/200081 Run-TryBot: Dan Scales Reviewed-by: Keith Randall --- src/runtime/testdata/testprog/deadlock.go | 58 +++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) (limited to 'src/runtime/testdata') diff --git a/src/runtime/testdata/testprog/deadlock.go b/src/runtime/testdata/testprog/deadlock.go index 9ca0fc344f..105d6a5faa 100644 --- a/src/runtime/testdata/testprog/deadlock.go +++ b/src/runtime/testdata/testprog/deadlock.go @@ -24,6 +24,7 @@ func init() { register("RecursivePanic", RecursivePanic) register("RecursivePanic2", RecursivePanic2) register("RecursivePanic3", RecursivePanic3) + register("RecursivePanic4", RecursivePanic4) register("GoexitExit", GoexitExit) register("GoNil", GoNil) register("MainGoroutineID", MainGoroutineID) @@ -31,6 +32,8 @@ func init() { register("GoexitInPanic", GoexitInPanic) register("PanicAfterGoexit", PanicAfterGoexit) register("RecoveredPanicAfterGoexit", RecoveredPanicAfterGoexit) + register("RecoverBeforePanicAfterGoexit", RecoverBeforePanicAfterGoexit) + register("RecoverBeforePanicAfterGoexit2", RecoverBeforePanicAfterGoexit2) register("PanicTraceback", PanicTraceback) register("GoschedInPanic", GoschedInPanic) register("SyscallInPanic", SyscallInPanic) @@ -146,6 +149,17 @@ func RecursivePanic3() { panic("first panic") } +// Test case where a single defer recovers one panic but starts another panic. If +// the second panic is never recovered, then the recovered first panic will still +// appear on the panic stack (labeled '[recovered]') and the runtime stack. +func RecursivePanic4() { + defer func() { + recover() + panic("second panic") + }() + panic("first panic") +} + func GoexitExit() { println("t1") go func() { @@ -237,6 +251,50 @@ func RecoveredPanicAfterGoexit() { runtime.Goexit() } +func RecoverBeforePanicAfterGoexit() { + // 1. defer a function that recovers + // 2. defer a function that panics + // 3. call goexit + // Goexit runs the #2 defer. Its panic + // is caught by the #1 defer. For Goexit, we explicitly + // resume execution in the Goexit loop, instead of resuming + // execution in the caller (which would make the Goexit disappear!) + defer func() { + r := recover() + if r == nil { + panic("bad recover") + } + }() + defer func() { + panic("hello") + }() + runtime.Goexit() +} + +func RecoverBeforePanicAfterGoexit2() { + for i := 0; i < 2; i++ { + defer func() { + }() + } + // 1. defer a function that recovers + // 2. defer a function that panics + // 3. call goexit + // Goexit runs the #2 defer. Its panic + // is caught by the #1 defer. For Goexit, we explicitly + // resume execution in the Goexit loop, instead of resuming + // execution in the caller (which would make the Goexit disappear!) + defer func() { + r := recover() + if r == nil { + panic("bad recover") + } + }() + defer func() { + panic("hello") + }() + runtime.Goexit() +} + func PanicTraceback() { pt1() } -- cgit v1.3