-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
tokio-channel: fix race with dropping Receiver #601
Conversation
Thank you. |
Previously, when dropping `Receiver`, there was a race condition that permitted messages to stay in the channel indefinitely. This patch fixes the race. Now, when `Receiver` is dropped, it is guaranteed that no messages will be stuck in the channel (assuming there are no further bugs, of course).
I have applied a fix cc @seanmonstar |
* 100ms is not enough for CI * Spinning is not good.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The fix looks good to me, thanks @carllerche!
self.dec_num_messages(); | ||
|
||
// Return the message | ||
return Ok(Async::Ready(msg)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moving this code to next_message
makes sense to me 👍
Fixed upstream rust-lang/futures-rs#1241 |
This branch adds a stress test to
tokio-channel
for a race conditionthat can occur when the
Receiver
end of a channel is dropped as theSender
end callstry_send
, leaving the sent data in the queue. Thisis based on the repro written by @simmons for
rust-lang/futures-rs#909; since the
tokio-channel
code isbased on the
futures-rs
channel implementation, the issue exists hereas well.
Note that since this branch does not resolve the issue, the stress
test currently fails.
Signed-off-by: Eliza Weisman eliza@buoyant.io