Skip to content

Commit

Permalink
Explicitly flush stdio in the nailgun client. (pantsbuild#11383)
Browse files Browse the repository at this point in the history
### Problem

In some cases when `pantsd` is used, stdio is not flushed during successful runs (and failing runs as well, but successful is more unusual). See pantsbuild#11335.

### Solution

Of the buffers listed in pantsbuild#11335 (comment), only the final step (writing to the `tokio::io::std*` handles) does not have a "guaranteed" flush when `Drop` runs, or when the channel is fully consumed. Usually the stdio file handles (`sys::io::std*`) would be torn down on `Drop` during a clean exit of a Rust process, and although their docs claim not to be buffered, their internal implementation is [line buffered by default](https://doc.rust-lang.org/src/std/io/stdio.rs.html#489).

But Pants clients exit with `sys.exit`, and that likely bypasses `Drop` for statics. So: explicitly flush `tokio::io::std*` before exiting the client code.

[ci skip-build-wheels]
  • Loading branch information
stuhood authored Dec 23, 2020
1 parent 8fe3a1e commit e25327a
Showing 1 changed file with 5 additions and 3 deletions.
8 changes: 5 additions & 3 deletions src/rust/engine/nailgun/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ use std::net::Ipv4Addr;
use tokio::io::AsyncWriteExt;

use futures::channel::mpsc;
use futures::{SinkExt, Stream, StreamExt};
use futures::{try_join, SinkExt, Stream, StreamExt};

pub enum NailgunClientError {
PreConnect(String),
Expand All @@ -59,12 +59,12 @@ async fn handle_client_output(
match output {
Some(ChildOutput::Stdout(bytes)) => {
stdout.write_all(&bytes).await.map_err(|err| {
NailgunClientError::PostConnect(format!("Failed to flush stdout: {}", err))
NailgunClientError::PostConnect(format!("Failed to write to stdout: {}", err))
})?
},
Some(ChildOutput::Stderr(bytes)) => {
stderr.write_all(&bytes).await.map_err(|err| {
NailgunClientError::PostConnect(format!("Failed to flush stderr: {}", err))
NailgunClientError::PostConnect(format!("Failed to write to stderr: {}", err))
})?
},
None => break,
Expand All @@ -84,6 +84,8 @@ async fn handle_client_output(
}
}
}
try_join!(stdout.flush(), stderr.flush())
.map_err(|e| NailgunClientError::PostConnect(format!("Failed to flush stdio: {}", e)))?;
Ok(())
}

Expand Down

0 comments on commit e25327a

Please sign in to comment.