Bounded channel fully deadlocked when sender is forgotten
sbarral opened this issue · 4 comments
A blocked sender loosing interest (i.e. kept alive but no longer polling) leads to a full deadlock: other blocked senders are never re-scheduled when messages are dequeued. This behaviour is surprising and quite the footgun since there is theoretically no obligation to poll a future to completion. AFAIK this is also at odd with what other channels do (tokio::mpsc even goes farther and prevents a deadlock if more senders are forgotten than the nominal capacity -- but this is arguably a less likely scenario).
Fortunately the fix looks trivial: use Event::notify_additional instead of Event::notify and get rid of the extra notification currently sent when a sender is unblocked.
Thanks for the report and investigation! I would accept a PR to fix this.
Funnily enough I have just noticed that PR #16 (which was never merged) does exactly what I suggested above, so in theory the PR already exists :-)
Its motivations were different though, and the original author of this crate was reluctant, commenting that this may not be a clear win performance-wise. I actually agree: performance will be degraded when the channel is hammered by many senders. But I think we should bite the bullet and accept a degradation in performance in such cases for the sake of correctness. AFAIK, flume, postage and tokio all notify one additional sender each time a slot is freed and thus avoid the potential deadlock.
If you prefer a fresh PR with commit messages more focused on the correctness aspect, please let me know.
Also, I failed to mention this but a similar issue exists when receivers are forgotten, and PR #16 fixes this as well.
So I went ahead with a fresh PR since the old one had conflicts and anyway I wanted to add tests to confirm that it closes this issue.