go: runtime: panic + recover can cancel a call to Goexit

The documentation for runtime.Goexit says (emphasis mine):

Goexit terminates the goroutine that calls it. No other goroutine is affected. Goexit runs all deferred calls before terminating the goroutine. Because Goexit is not a panic, any recover calls in those deferred functions will return nil.

However, this is empirically not the case if one of the deferred functions itself triggers a panic: that panic can be recovered, and that recover cancels the termination of the goroutine.

See https://play.golang.org/p/7VrxPDByUNT for a minimal illustration.

Found while investigating #29207.

CC @aclements @randall77

About this issue

  • Original URL
  • State: closed
  • Created 6 years ago
  • Reactions: 2
  • Comments: 49 (29 by maintainers)

Most upvoted comments

Should this following program print should be unreachable or not. I tried tip, it prints it.

package main

import "fmt"
import "runtime"

func f() {
	defer func() {
		recover()
	}()
	panic("will cancel Goexit but should not")
	runtime.Goexit()
}

func main() {
	c := make(chan struct{})
	go func() {
		defer close(c)
		f()
		fmt.Println("should be unreachable")
	}()
	<-c
}

This is exactly performing to spec and is unrelated to the current bug. https://golang.org/ref/spec#Handling_panics and https://go-review.googlesource.com/c/go/+/189377

The panic() call starts a panic sequence. By spec, the function that called panic will never continue running normally (hence you will never reach the Goexit). If a recover occurs in a defer directly called by the panicking sequence, then the panic will be recovered, and (after finishing any remaining defers in that frame), execution of the goroutine will continue in the caller of that frame (which in this case is f).

There is another related question: should a Goexit call save the a panicking goroutine from crashing? The current gc and gccgo both do this. Personally, I think it is not very reasonable.

package main

import "runtime"
import "time"


func main() {
	go func() {
		defer runtime.Goexit() // [edit]: add the "defer"
		panic(1) // will not crash the program
	}()
	
	for {time.Sleep(100000)}
}

In fact, the key here is, if Goexit creates a panic, in @bcmills’s example, the panic created in Goexit is suppressed by the new panic created in defer panic("cancelled Goexit!"), which is consequently recovered. (If the suppression doesn’t happen, the the recover will not happen too, for the panic created in Goexit is unrecoverable and will not cause crashing.) So, if the issue needs to be fixed, then we should avoid letting the new panic created in defer panic("cancelled Goexit!") suppress the special panic created in Goexit.