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 <danscales@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Keith Randall <khr@golang.org>
Trust: Dan Scales <danscales@google.com>
This commit is contained in:
Dan Scales 2021-01-25 17:51:03 -08:00
parent 8cfa01943a
commit a5a5e2c968
4 changed files with 113 additions and 29 deletions

View file

@ -1000,37 +1000,42 @@ func gopanic(e interface{}) {
}
atomic.Xadd(&runningPanicDefers, -1)
if done {
// Remove any remaining non-started, open-coded
// defer entries after a recover, since the
// corresponding defers will be executed normally
// (inline). Any such entry will become stale once
// we run the corresponding defers inline and exit
// the associated stack frame.
d := gp._defer
var prev *_defer
for d != nil {
if d.openDefer {
if d.started {
// This defer is started but we
// are in the middle of a
// defer-panic-recover inside of
// it, so don't remove it or any
// further defer entries
break
}
if prev == nil {
gp._defer = d.link
} else {
prev.link = d.link
}
newd := d.link
freedefer(d)
d = newd
// Remove any remaining non-started, open-coded
// defer entries after a recover, since the
// corresponding defers will be executed normally
// (inline). Any such entry will become stale once
// we run the corresponding defers inline and exit
// the associated stack frame.
d := gp._defer
var prev *_defer
if !done {
// Skip our current frame, if not done. It is
// needed to complete any remaining defers in
// deferreturn()
prev = d
d = d.link
}
for d != nil {
if d.started {
// This defer is started but we
// are in the middle of a
// defer-panic-recover inside of
// it, so don't remove it or any
// further defer entries
break
}
if d.openDefer {
if prev == nil {
gp._defer = d.link
} else {
prev = d
d = d.link
prev.link = d.link
}
newd := d.link
freedefer(d)
d = newd
} else {
prev = d
d = d.link
}
}