Skip to content

Commit

Permalink
Auto merge of #6515 - ehuss:fix-fix-diag-race, r=alexcrichton
Browse files Browse the repository at this point in the history
Fix a very minor race condition in `cargo fix`.

There is a very rare race where a "fix" message was getting posted *after* `Message::Finish`, and thus being dropped without being printed. This is caused by the diagnostic server disconnecting the client before posting `Message::FixDiagnostic`. The solution is to keep the client on the line until after the message is posted.

Fixes some errors in `fixes_missing_ampersand` seen in CI:
https://travis-ci.org/rust-lang/cargo/jobs/474384359
https://travis-ci.org/rust-lang/cargo/jobs/429809800

I also moved the `done` check to the beginning of the loop because every time the server was shut down it was needlessly trying to parse an empty string (and failing).
  • Loading branch information
bors committed Jan 3, 2019
2 parents dd97f8b + 7584dce commit d4a04da
Showing 1 changed file with 14 additions and 5 deletions.
19 changes: 14 additions & 5 deletions src/cargo/util/diagnostic_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -258,14 +258,23 @@ impl RustfixDiagnosticServer {

fn run(self, on_message: &dyn Fn(Message), done: &AtomicBool) {
while let Ok((client, _)) = self.listener.accept() {
let client = BufReader::new(client);
match serde_json::from_reader(client) {
Ok(message) => on_message(message),
Err(e) => warn!("invalid diagnostics message: {}", e),
}
if done.load(Ordering::SeqCst) {
break;
}
let mut client = BufReader::new(client);
let mut s = String::new();
if let Err(e) = client.read_to_string(&mut s) {
warn!("diagnostic server failed to read: {}", e);
} else {
match serde_json::from_str(&s) {
Ok(message) => on_message(message),
Err(e) => warn!("invalid diagnostics message: {}", e),
}
}
// The client should be kept alive until after `on_message` is
// called to ensure that the client doesn't exit too soon (and
// Message::Finish getting posted before Message::FixDiagnostic).
drop(client);
}
}
}
Expand Down

0 comments on commit d4a04da

Please sign in to comment.