Skip to content
This repository has been archived by the owner on Nov 5, 2018. It is now read-only.

Possible deadlock footgun with lost consumer #55

Closed
vorner opened this issue Jun 12, 2018 · 4 comments · Fixed by #106
Closed

Possible deadlock footgun with lost consumer #55

vorner opened this issue Jun 12, 2018 · 4 comments · Fixed by #106

Comments

@vorner
Copy link

vorner commented Jun 12, 2018

I noticed the send on the sender no longer returns a result. I find this a little bit troubling. Consider the producer-consumer design pattern. Originally, the producer would be doing something like sender.send(stuff).unwrap().

Now, if the consumer terminated due to an error (for example by panicking), the panic would propagate, or would require the producer to handle it somehow. The application would probably terminate by an error. However, in the current implementation, the producer will deadlock (in case of bounded channel, in case of unbounded, it'll just keep eating memory forever), not bringing attention to the problem automatically by eg. crashing.

The previous version was a little bit more typing, but seemed to prevent these kinds of subtle bugs.

@ghost
Copy link

ghost commented Jun 12, 2018

This aspect of the new design is unfortunate, but intentional. It was a really difficult decision to make. See #39 for the discussion and rationale.

The good news is that it's still possible to simulate the old interface:
https://github.com/crossbeam-rs/crossbeam-channel/blob/master/examples/mpsc.rs

@ghost ghost added the question label Jun 12, 2018
@vorner
Copy link
Author

vorner commented Jun 12, 2018

Oh, I see there's some point in the unidirectionality and why bidirectional closing can be a footgun too.

Anyway, two things:

  • Someone in the thread mentioned that if a deadlock is possible to detect (I'm not sure if it is possible), it would still make sense to panic from within the .send. I only skimmed the text, but I think nobody reacted to that one. Even opportunistic detection of a bug could be helpful (and deadlocking is hardly a desired behaviour).
  • It would be nice to actually mention the case with no receivers in the documentation ‒ maybe it looks silly, but I actually had to write some code to see if it would start throwing the items out or deadlock.

@ghost
Copy link

ghost commented Sep 16, 2018

Regarding deadlocks, are you suggesting to panic in situations like the following?

  • Sending into a full channel without receivers.
  • Receiving the second message from channel::after (one message is delivered only and the channel is never closed).
  • Selecting over no operations (currently this just blocks forever).
  • Selecting over operations that will never complete.

@vorner
Copy link
Author

vorner commented Sep 17, 2018

Yes, something like that… but being careful about it. I mean, trying to receive the second one on after will definitely deadlock, but if it's part of select, something else might succeed, so it is not necessarily a bug and can probably happen as a corner case of something.

@ghost ghost added the enhancement label Sep 17, 2018
@ghost ghost closed this as completed in #106 Nov 4, 2018
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging a pull request may close this issue.

1 participant