go: spec: clarify when calling recover stops a panic

What version of Go are you using (`go version`)?

go version go1.13 linux/amd64

Does this issue reproduce with the latest release?

Yes

What did you do?

package main

import "fmt"

func demo() {
	defer func() {
		defer func() {
			// recover panic 2
			fmt.Println("panic", recover(), "is recovered")
		}()

		// recover panic 1
		defer fmt.Println(" (done).")
		defer recover()
		defer fmt.Print("To recover panic 1 ...")

		defer fmt.Println("now, two active panics coexist")
		panic(2) // If this line is commented out, then the program will not crash.
	}()
	panic(1)
}

func main() {
	demo()
}

What did you expect to see?

Not crash.

What did you see instead?

Crashes.

About this issue

Original URL
State: open
Created 5 years ago
Reactions: 2
Comments: 19 (12 by maintainers)

Commits related to this issue

add json test, and fix recover() in defer as per https://github.com/golang/go/issues/34530 — committed to KarpelesLab/contexter by MagicalTux 2 years ago

Most upvoted comments

Thanks for all the examples and discussion. I missed that there was a defer recover() even in the first example. @ianlancetaylor gives a very good explanation of the current behavior of defer recover(). Overall, the meaning of defer recover() isn’t intuitively obvious and its actual behavior with the current implementation is confusing. So, we are thinking about banning the use of defer recover() (since it seems likely to be only useful for test code). Instead of defer recover(), you should always at least use defer func() { recover() }() (and of course, it is always recommended to look at the return code of recover()).

With respect to talking about (and spec’ing out) the behavior of panic/recover/recursive panics, it will be best not to use defer recover(), since that will just create more cases that tend to confuse the other issues.

danscales on Oct 2, 2019

I think this is a really interesting test case. Thanks for reporting it, @go101.

On my phone, so I haven’t looked at the Go spec wording or @danscales’s suggestion recently, so I’m not sure the right behaviour. From memory though, I can see arguments either way.

mdempsky on Sep 30, 2019

Yes, the Go language spec is very vague and imprecise on panic/recover and I am hoping to fix it:

https://go-review.googlesource.com/c/go/+/189377

but there’s not quite consensus yet.

I don’t think there is any disagreement on the behavior of your example (unlike the abort behavior mentioned in #29226 ). A recover only recovers a panic if it is called directly in a defer function that is directly invoked as part of the panicking process of that panic. A recover does not apply and returns nil if it is not called directly in a defer function or if it is called from a defer that was not directly invoked by the panic sequence of the panic (i.e. is nested inside some other defer or function called by the defer).

Your second example seems to be a special little bug/quirk of doing ‘defer recover()’. The detection mechanism in the implementation considers that recover is called directly in the containing function, so that recover does apply. Note that recover does not happen if you do

  defer func() {
    recover()
  }

I think the first thing in both these bugs are to get agreement on the spec - both specifying the current behavior that people depend on and/or would generally agree on, and also the behavior (like the abort behavior in #29226 ) that has been there for a while, but has never been specified and people might like to change.

danscales on Sep 26, 2019