What if panic(a) where a is nil was a no op? That is, what if panic(a)
were the same as writing
if a != nil {
panic(a)
}
Currently, panic'ing on a nil interface is unrecoverable:
# cat uhoh.go
package main
func main() {
defer func() {
if x := recover(); x != nil {
println("recovered")
} else {
println("failed to recover")
}
}()
var a interface{} = nil
panic(a)
}
# ./8.out
failed to recover
#
And for obvious reasons.
(From here on, I will use the shorthand panic(nil), though not legal
code in itself, to refer to scenarios such as the one above).
Aside:---
I brought this issue up in the thread where Rob Pike originally
announced panic and at that time suggested a way to avoid panic(nil)
was for recover's signature be recover() (err interface{}, recovered
bool) so that one would write
if x, recovered := recover(); recovered {
//deal with x
}
Rob said that this was too fussy, and after writing many a recover(),
I am inclined to agree.
---
panic(nil) is not presently an issue, merely a bug in the code using
panic. Should Go at any time include a dynamic loading, however, it
would be possible for a malicious or poorly vetted dynamically loaded
module to take out the host program. Now, the other way around this,
that I see, is for the dynamic loader to recover any panics and report
them to the host program by some means, but I can't imagine this
fitting well into the language or being anything other than clunky.
Perhaps I am wrong.
Of course, my current suggestion for panic(nil) being a no-op
certainly has its own problems, which is why I am hesitant to call
this even a suggestion. I have my reservations about this hypothetical
"fix" that I'll outline below.
The most obvious problem with panic(nil) being a no-op is that when
you request a panic you expect your program to panic and now it's more
difficult to see that panic'ing on a nil interface is the cause of
your problems. Worse yet, your program might continue on being
imperceptibly wrong. This is my primary reservation.
Aside from that--and I honestly do not know whether I am for or
against this--is that if it could be relied on that panic(nil) was a
no-op, then code like the following would be allowed:
//obviously this is deep within some complicated subsystem and the
panic would be
//recovered and returned as an error message at the package-boundary.
resc, err := AcquireVitalResource()
panic(err) //<-- panics iff there is an error
defer resc.Release()
The part of me that is extremely lazy and too clever for my own good
finds this an appealing idiom. It also strikes me as a bit too
implicit, magical, and mysterious; then again, what idiom isn't to
someone unaccustomed to it? Of course, it's not that difficult to
write
if err != nil { panic(err) }
when you mean to. You would have to anyway if you wished to panic on a
different object, which in most cases you should. Moreover, at this
point panic basically turns into assert, which is a specific no in the
language design FAQ. I think, that all in all, I am against this usage
which would be implicitly granted by making panic(nil) a no-op.
Another way around panic(nil) is for panic to test if the panic'd
value is nil and if it is and instead panic on some special value like
runtime.NilPanic or some such, so that if you are in a position to
worry about foreign code taking you out there is the option to test
for it and everyone else may happily ignore the scenario and let their
program crash naturally so they can see the bug. Perhaps this is the
better solution, if in fact anyone other than me considers this a
problem, but I had to write everything above before it occurred to me
so I may as well present both ideas.
Russ
I hadn't even considered and/or forgot that. Very interesting.
> If that's a problem in your program, don't call panic(nil).
> I used panic(nil) recently and it did exactly what I needed.
I'm still a bit weary about a panic that to a typical "if x :=
recover(); x != nil {" does not appear to be a panic even if recover
recovers it succesfully. Namely:
func F() (t *T, e *E) {
defer func() {
if x := recover(); x != nil {
e = x
} else {
t.finalize() //and this method actually derefs its reciever
}
}()
could_panic_nil_accidentially()
t = new_t()
etc()
return
}
If could_panic_nil_accidentially does panic nil, then t is nil but
looking at this code why t is nil is hidden in plain sight.
How did you use panic(nil) recently? I think I need an example of its
utility to ease my mind.
Regardless, though, I think the documentation should mention this
corner case so that it wouldn't be so confusing if someone found
themselves in a situation like the example above.
If we're going to posit buggy code, anything is possible.
> If could_panic_nil_accidentially does panic nil, then t is nil but
> looking at this code why t is nil is hidden in plain sight.
>
> How did you use panic(nil) recently? I think I need an example of its
> utility to ease my mind.
I had a complex http handler that prepared HTML output
into a buffer but could panic to signal that an error happened,
unwind the processing stack, and send an error page.
The code was basically
defer func() {
if err := recover(); err != nil {
req.internalError(err)
}
}()
Sometimes (like when you send a redirect instead of HTML)
you want to stop, unwind the processing stack, and not send
an error. For that panic(nil) worked perfectly.
In the end I realized that what I was writing didn't really
need panic anyway (it was equally easy to return),
so I threw it away. But it worked while it was there.
Russ
The way to handle this is not pretty, but its fairly simple. Just
keep track of it yourself like this:
func R(f func()) (e os.Error) {
var recovered = true
defer func() {
if x := recover(); recovered {
e = os.NewError(fmt.Sprint(x))
}
}()
f()
recovered = false
return
}
A complete example is attached.
Even simpler: when it matters, don't call panic(nil).
Russ
True enough, I only meant to demonstrate a case that would yield a
baffling debugging session.
>> How did you use panic(nil) recently? I think I need an example of its
>> utility to ease my mind.
*snip*
> Sometimes (like when you send a redirect instead of HTML)
> you want to stop, unwind the processing stack, and not send
> an error. For that panic(nil) worked perfectly.
>
> In the end I realized that what I was writing didn't really
> need panic anyway (it was equally easy to return),
> so I threw it away. But it worked while it was there.
Hmm, that makes sense. I did something similiar not long back, except
I had to pass information to the top with the result, but the
principle is the same. I think though that even if I needn't pass
information up the stack that I would have, personally, used a
sentinel value or type, if for no other reason than out of sheer
paranoia.
couldn't you use runtime.Goexit for that?
personally, i think that guaranteeing that any
panic is recoverable is good - if some code accidentally
does a panic on an uninitialised variable, it's
going to be a really hard bug to track down.
since you can, i think, use Goexit for the
functionality you were after, i think it would be
reasonable if panic(nil) turned into panic("nil panic")
or something similar.
Only if your function is the top level of a goroutine. You could
simply reserve a special value or have a special NotAnError type.
> personally, i think that guaranteeing that any
> panic is recoverable is good - if some code accidentally
> does a panic on an uninitialised variable, it's
> going to be a really hard bug to track down.
As Russ pointed out, you recover either way but you can't tell you
recovered if you panicked on a nil, which I agree could lead to some
difficult to trace bugs.
> since you can, i think, use Goexit for the
> functionality you were after, i think it would be
> reasonable if panic(nil) turned into panic("nil panic")
> or something similar.
Were such a step to be taken it would have to be special type as
otherwise you run into the same problem if you type switch on the
string without checking if it's not "nil panic". But perhaps I'm being
overly fussy again.
yes, sorry, i'm stupid - Goexit is not recoverable, but
panic(nil) is, even though both cause recover() to return nil.
> Were such a step to be taken it would have to be special type as
> otherwise you run into the same problem if you type switch on the
> string without checking if it's not "nil panic". But perhaps I'm being
> overly fussy again.
i'm not sure you have the same problem - it's just like the runtime
raising an error for array out of bounds or invalid pointer dereference.
if i was to raise an error but not cause an error to be returned,
i would, as you suggest, use a special type or value for that.
using nil seems kind of "clever", in a not good way.
BTW, interestingly, panic(nil) itself doesn't compile ("use of
untyped nil"): you need panic(interface{}(nil)) or something similar.
no: i wanted to unwind only to the recover block.
when that returns the http handler returns.
the whole goroutine can't die.
(in fact i think runtime.Goexit should probably go.)
russ
This was issue 990.
Russ