From a5a5e2c968eb14335f4e46606d8edfbdbdcea728 Mon Sep 17 00:00:00 2001 From: Dan Scales Date: Mon, 25 Jan 2021 17:51:03 -0800 Subject: runtime: make sure to remove open-coded defer entries in all cases after a recover We add entries to the defer list at panic/goexit time on-the-fly for frames with open-coded defers. We do this so that we can correctly process open-coded defers and non-open-coded defers in the correct order during panics/goexits. But we need to remove entries for open-coded defers from the defer list when there is a recover, since those entries may never get removed otherwise and will get stale, since their corresponding defers may now be processed normally (inline). This bug here is that we were only removing higher-up stale entries during a recover if all defers in the current frame were done. But we could have more defers in the current frame (as the new test case shows). In this case, we need to leave the current defer entry around for use by deferreturn, but still remove any stale entries further along the chain. For bug 43921, simple change that we should abort the removal loop for any defer entry that is started (i.e. in process by a still not-recovered outer panic), even if it is not an open-coded defer. This change does not fix bug 43920, which looks to be a more complex fix. Fixes #43882 Fixes #43921 Change-Id: Ie05b2fa26973aa26b25c8899a2abc916090ee4f5 Reviewed-on: https://go-review.googlesource.com/c/go/+/286712 Run-TryBot: Dan Scales TryBot-Result: Go Bot Reviewed-by: Matthew Dempsky Reviewed-by: Keith Randall Trust: Dan Scales --- src/runtime/testdata/testprog/deadlock.go | 39 +++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) (limited to 'src/runtime/testdata') diff --git a/src/runtime/testdata/testprog/deadlock.go b/src/runtime/testdata/testprog/deadlock.go index 105d6a5faa..781acbd770 100644 --- a/src/runtime/testdata/testprog/deadlock.go +++ b/src/runtime/testdata/testprog/deadlock.go @@ -25,6 +25,7 @@ func init() { register("RecursivePanic2", RecursivePanic2) register("RecursivePanic3", RecursivePanic3) register("RecursivePanic4", RecursivePanic4) + register("RecursivePanic5", RecursivePanic5) register("GoexitExit", GoexitExit) register("GoNil", GoNil) register("MainGoroutineID", MainGoroutineID) @@ -160,6 +161,44 @@ func RecursivePanic4() { panic("first panic") } +// Test case where we have an open-coded defer higher up the stack (in two), and +// in the current function (three) we recover in a defer while we still have +// another defer to be processed. +func RecursivePanic5() { + one() + panic("third panic") +} + +//go:noinline +func one() { + two() +} + +//go:noinline +func two() { + defer func() { + }() + + three() +} + +//go:noinline +func three() { + defer func() { + }() + + defer func() { + fmt.Println(recover()) + }() + + defer func() { + fmt.Println(recover()) + panic("second panic") + }() + + panic("first panic") +} + func GoexitExit() { println("t1") go func() { -- cgit v1.3