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
net: Avoid race during io driver shutdown #2997
net: Avoid race during io driver shutdown #2997
Conversation
To avoid IO resources getting registered while the driver is shutting down,`state: AtomicUsize` is introduced. Fixes: tokio-rs#2924
loop { | ||
match self | ||
.state | ||
.compare_exchange(WAITING, REGISTERING, AcqRel, Acquire) | ||
{ | ||
Ok(_) | Err(REGISTERING) => break, | ||
Err(CLOSED) => return Err(io::Error::new(io::ErrorKind::Other, "reactor gone")), | ||
_ => unreachable!(), | ||
} |
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.
I have an uneasy feeling about this. Is it ok to spin like that here? Is there a better way to do it?
io.wake(Ready::ALL); | ||
}) | ||
let inner = self.inner.take().expect("reactor closed"); | ||
loop { |
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.
Generally spinning is not a great strategy as it will mess w/ schedulers.
Thanks for taking a stab at this 👍 The original issue ended up getting fixed as part of #2903. That said, my guess is we could improve the shutdown strategy more, but it will likely require rethinking some of how the i/o driver is structured. At this point, it probably isn't top priority. |
To avoid IO resources getting registered while the driver
is shutting down,
state: AtomicUsize
is introduced.Motivation
As described in #2924 IO resources can be added while the driver is shutting down, resulting in these resources never being woken.
Solution
An atomic state is added that should ensure no IO resources are added while the driver is shutting down. ]
Fix #2924