spec: panicking corner-case semantics

[mdempsky]

A few corner-cases regarding panicking that I noticed in the Go spec:

1) What happens if a panic occurs while a goroutine is already panicking?

Gc/gccgo seem to allow recursive panicking and recovering: http://play.golang.org/p/tBkwgyzmuT

But if a deferred function panics without any recovery, then the original panic is lost: http://play.golang.org/p/KqwGiWGMAx

2) The Go spec says "The return value of recover is nil if any of the following conditions holds: [...] recover was not called directly by a deferred function."

Gc/gccgo though seem to also have recover return nil in deferred functions if they were executed because of normal (i.e., non-panicking) function return: http://play.golang.org/p/a-fl_9Gga0

3) The Go spec says "Suppose a function G defers a function D that calls recover and a panic occurs in a function on the same goroutine in which G is executing. [...] If D returns normally, without starting a new panic, the panicking sequence stops."

Nit: I think the intention here is understood, but the wording could probably be improved somewhat. As quoted in item 2 above, the spec later explicitly refers to deferred functions that "directly" call recover. Omitting "directly" here suggests that indirect calls to recover should still stop the panicking sequence, but they'll return nil instead of the panic value.

[ianlancetaylor]

Unfortunately I don't understand what you are suggesting. To me the spec seems accurate and complete.

You seem to suggest that we should omit "directly," but that would be a clear change in behaviour. Indirect calls to recover do not stop the panicking sequence. They simply return nil and the panic continues.

[mdempsky]

@ianlancetaylor Are you referring to just item 3, or you think the spec is accurate and complete for the first two items I listed too?

Rephrasing my concern about the wording in item 3, currently the spec seems to admit this behavior for recover:

func recover() interface{} {
    // 1. Apply the "stop panicking" logic from the "The recover function allows a program [...]" paragraph.
    if ("called by function D [possibly indirectly?], which was deferred by panicking function G") {
        stopPanicking()
    }

    // 2. Apply the "return nil" logic from the next paragraph.
    if ("goroutine is not panicking" || "not *directly* called by a deferred function") {
        return nil
    }

    // 3. Continue with the previous paragraph and return the value from panicking.
    return valueFromPanicking()
}

The nit I see is the stop-panicking logic and the what-value-is-returned logic are separately described with slightly different wording.

I think by reorganizing the section a bit, it could be clearer that the correct behavior is something like:

func recover() interface{} {
    // 1. This case always return nil without doing anything.
    // (Note: No need to test for "not panicking", because that's just a special case.)
    if ("not *directly* called by a function D, deferred by a panicking function G") {
        return nil
    }

    // 2. Stop panicking and return the panic value.
    stopPanicking()
    return valueFromPanicking()
}

[ianlancetaylor]

Thanks, I think I see what you mean now. We should clarify that we only stop panicking for the cases where recover returns non-nil.

[gopherbot]

Change https://golang.org/cl/189377 mentions this issue: spec: specify the behavior of recover and recursive panics in more detail