Skip to content

Commit cc8c5a0

Browse files
authored
Ignore client messages after stopping the IO task (#1126)
This protects us from a subtle race condition: - The main task decides to stop the client task (e.g. because we've hit `IO_OUTSTANDING_MAX`) - It sends a stop request down the `tokio::sync::oneshot` channel - Then, it marks all jobs as skipped, which may result in discarding some of them - The IO task receives a message from the downstairs - It passes this message to the main task - This message is a reply to one of the jobs that we skipped - The main task gets mad
1 parent 1325543 commit cc8c5a0

File tree

1 file changed

+14
-6
lines changed

1 file changed

+14
-6
lines changed

upstairs/src/client.rs

+14-6
Original file line numberDiff line numberDiff line change
@@ -285,13 +285,21 @@ impl DownstairsClient {
285285
/// must the select expressions be cancel safe, but the **bodies** must also
286286
/// be cancel-safe. This is why we simply return a single value in the body
287287
/// of each statement.
288+
///
289+
/// This function will wait forever if we have asked for the client task to
290+
/// stop, so it should only be called in a higher-level `select!`.
288291
pub(crate) async fn select(&mut self) -> ClientAction {
289-
tokio::select! {
290-
d = self.client_task.client_response_rx.recv() => {
291-
match d {
292-
Some(c) => c.into(),
293-
None => ClientAction::ChannelClosed,
294-
}
292+
loop {
293+
let out = match self.client_task.client_response_rx.recv().await {
294+
Some(c) => c.into(),
295+
None => break ClientAction::ChannelClosed,
296+
};
297+
// Ignore client responses if we have told the client to exit (we
298+
// still propagate other ClientAction variants, e.g. TaskStopped).
299+
if self.client_task.client_stop_tx.is_some()
300+
|| !matches!(out, ClientAction::Response(..))
301+
{
302+
break out;
295303
}
296304
}
297305
}

0 commit comments

Comments
 (0)