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

sync: fix panic in broadcast::Receiver drop #3434

Merged
merged 2 commits into from Jan 20, 2021

Conversation

oguzbilgener
Copy link
Sponsor Contributor

Motivation

Fixes: #2533

I came across this problem in my program with multiple worker threads and an insufficient channel capacity. With a debugger, I noticed that self.next can be set a greater value than until by self.recv_ref() when it's called multiple times in the loop in drop.

Here is another program that reproduces the panic:
https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=d09c0b70aeb7187b87a01db544c6b821

Solution

  • Only call self.recv_ref when self.next is smaller than until.
  • Added a loom test that fails without this change.

This test case causes `broadcast::Receiver` to panic on drop
with "unexpected empty broadcast channel" when the tests are
executed with this command:

```
LOOM_MAX_PREEMPTIONS=1 RUSTFLAGS="--cfg loom" \
    cargo test --lib --release --features full -- --test-threads=1 --nocapture
```

Refs: tokio-rs#2533
`self.next` can be set to a value greater than `until`
when the channel overflows with messages but the drop implementation
does not account for it.

Fixes: tokio-rs#2533
@Darksonn Darksonn added A-tokio Area: The main tokio crate M-sync Module: tokio/sync labels Jan 16, 2021
Copy link
Contributor

@Darksonn Darksonn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The way I think this happens is if the channel has lagged, which causes next to jump by more than 1, and the way it can jump to something larger than until is if something else sent while we were in the while loop.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate M-sync Module: tokio/sync
Projects
None yet
2 participants