Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Propagate panics cleanly #7

Open
madsmtm opened this issue Oct 28, 2024 · 1 comment
Open

Propagate panics cleanly #7

madsmtm opened this issue Oct 28, 2024 · 1 comment

Comments

@madsmtm
Copy link

madsmtm commented Oct 28, 2024

If the "sending" part panics and drops Sender, we get a double panic and a crash.

Something else should probably be done here, maybe set a flag (i.e. poison the future), and panic on next poll?

@drewcrawford
Copy link
Owner

So I think at the end of the day dropping without sending is a programmer error. In Rust we usually panic on those (Swift takes a bit of a different view on this point – not sure if that's part of the discussion or not).

Then we get to a question that does not arise in Swift with its single-type approach: Was that programmer error on the sending side or on the receiving side, or both?

In one of my earlier designs, Receiver had an implicit Result type, and would automatically get the Err side when Sender drops, which one could .recv().expect(), or not, etc. So an API much like oneshot. But I found a few problems with that approach:

  1. In (most? basically all?) cases the way to solve the misuse is either "dig up Sender and send a value", "dig up Sender and stop it from being dropped", and it is easier to dig up Sender when you have a Sender-side backtrace than a receiver-side one.
  2. Forcing Result means you get Result whether you like it or not. No implicit Result type means you can implement one on top if you want, not if you don't.
  3. From a certain view, it's desireable for the receiver thread to go off and do something else. On one hand, the future will never resolve, and that is a problem. On the other hand, maybe the receiver thread is polling 10 futures, and it can go on to do 9 out of the 10, which is arguably better than none of of 10.

Those were persuasive to me that it is strictly better to blow up Sender than receiver. If we leave off item 3, which I think is the weakest, it could make sense to poison Receiver as well.

But that still leaves your double-panic problem. One idea is to check thread::panicking() in Sender::drop. I'd guess it's no great loss to just let misuse slip through if that seems to be caused by a different panic (presumably the best developer experience to fix that misuse was to fix the original panic).

I am a little unsure how safe that assumption is, in the dark corners of "fully generalized" panic machinery. For example, could it be that: a) thread::panicking()==true, b) but don't worry about it, someone wrote a catch_unwind to smooth that over, c) but in the process of doing that they created a continuation misuse, which is d) unreported from step a?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants