It is not clear whether destructors in general should panic. However, JoinGuard in the standard library currently panics on drop to propagate child thread's panics. I am not sure whether this is the exception or the rule.
Moreover, it currently causes double panics, which is probably not desired.
Should destructors even be allowed to panic?
JoinGuard needs to panic in that situation, to solve a memory safety concern with exceptions, which stems from the same issue of panics and destructors: We can't come back from a scoped thread if the destructors didn't run properly.
However, don't you think it should be best practice to check if currently panicking, and panic but never double-panic?
I don't think that it's best practice to squelch errors in destructors if a different error has occured already. In my opinion, errors should be loud and obvious. If a double-panic becomes a problem, the way I'd try to solve it would be by introducing Results at the appropiate interfaces.
Do I recall it correctly that the second panic will obscure the first one? If you're unwinding from a failed assertion, it's not very helpful.
AFAIK the second panic will cause the process to abort.
I mean, you lose the first panic message and the backtrace, right? That would make it harder to get to the cause.
What sort of "allowed" are you envisioning here? Something the compiler would statically check to ensure that panic!() is never called (this would require an effect system to be even slightly usable, I believe), or just a strong guideline to programmers "destructors shouldn't panic"?
My intention is just to propose a guideline. I am not sure if JoinGuard
is going to be the only exception.
I believe destructors should not panic because of double-panic, though there may be some exceptions. Double-panic is never acceptable.
[quote="brson, post:9, topic:847"]
Double-panic is never acceptable.
[/quote]If so, should the assert macros check for that so we don't have to?
Is there anything assert could do about it that is not panicking?
Fair enough, I didn't think this through.
The debug asserts on the other hand arguably are optional. What if they just ignored the problem in this case?
We already have one case where panic in the destructor is necessary (JoinGuard). Given that we are a language that encourages implementation of concurrency primitives in library code, that solution will be required in other places as well (scoped Futures?) and so on.
I think it is best we find an error reporting strategy -- I do not think any errors should be squeched.
I think we need to:
Yes. They could simply not panic. But you see where this is going, I think "double panics are never acceptable" is a bad rule, because it squelches errors like, not panicking on assert! etc.
That being said, JoinGuard does indeed not need to double-panic. The only reason it needs to panic is so the parent thread can't do memory-unsafe things, but since it's already panicking that is already being taken care of.
Execution can't continue past the assert! statement if it fails: invariants are violated and latter code will be incorrect. Panicking is essentially the weakest way to stop execution at an arbitrary point. I don't know of any other way to handle this in a generic assert (or other general purpose thing that may panic): it seems reasonable to surround the contents of JoinGuard with a check of std::thread::panicking, but definitely not reasonable to do this with the code that assert! expands to.
Exactly this. That is why I made that example and wanted to show that the rule "double panics are never acceptable" is not a good rule to adhere. It's true that they should be avoided where it's possible where no invariants are violated.
Here is an alternative:
create a new strong_assert!() macro that should be used only near unsafe code
make the old assert!() do nothing if panicking() returns true
make the new strong_assert!() panic without checking panicking() (if it is already panicking, it will abort by double-panicking)