Skip to content

Commit

Permalink
fix(trace_pipe): don't close file prematurely
Browse files Browse the repository at this point in the history
I noticed trace pipe logging stopped working and I tracked down the
issue to the original file descriptor being closed too soon (when
returning from open_trace_pipe). Turns out AsyncFd is not taking
ownership the the original file and we must keep it around until we're
done reading)
  • Loading branch information
MatteoNardi committed Jul 27, 2022
1 parent 840612f commit 0e01450
Showing 1 changed file with 17 additions and 9 deletions.
26 changes: 17 additions & 9 deletions bpf-common/src/trace_pipe.rs
Expand Up @@ -17,16 +17,16 @@ pub struct StopHandle(oneshot::Sender<()>);

pub async fn start() -> StopHandle {
let (tx, rx) = oneshot::channel();
if let Some(mut file) = open_trace_pipe().await {
tokio::spawn(async move {
tokio::spawn(async move {
if let Some((mut async_fd, _open_file)) = open_trace_pipe().await {
log::info!("Logging events from {}", PATH);

tokio::pin!(rx);
loop {
let mut buf = BytesMut::with_capacity(512);
let file_event = tokio::select! {
// wait for a new event
f = file.read_buf(&mut buf) => f,
f = async_fd.read_buf(&mut buf) => f,
// exit when stop handle is dropped
_ = &mut rx => return,
};
Expand All @@ -36,28 +36,36 @@ pub async fn start() -> StopHandle {
}
print_buffer(&buf[..]);
}
});
}
}
});

StopHandle(tx)
}

async fn open_trace_pipe() -> Option<AsyncFd> {
/// Open the trace pipe file and returns:
/// - An AsyncFd which can be used to read asynchronously from it.
/// NOTE: we can't just use tokio::fs::File because it uses blocking IO on
/// a different thread. If we do, Ctrl-C won't quit the application since
/// we're still stuck reading this file.
/// - The original tokio::fs::File is returned and must not be dropped until
/// we're done reading from the async fd. Dropping it would close the FD,
/// resulting in EBADFD errors when trying to read.
async fn open_trace_pipe() -> Option<(AsyncFd, File)> {
let file = match File::open(PATH).await {
Ok(file) => file,
Err(e) => {
log::warn!("Error opening {}: {:?}", PATH, e);
return None;
}
};
let file = match AsyncFd::try_from(file.as_raw_fd()) {
Ok(file) => file,
let async_fd = match AsyncFd::try_from(file.as_raw_fd()) {
Ok(async_fd) => async_fd,
Err(e) => {
log::warn!("Error opening {} as non-blocking: {:?}", PATH, e);
return None;
}
};
Some(file)
Some((async_fd, file))
}

fn print_buffer(buf: &[u8]) {
Expand Down

0 comments on commit 0e01450

Please sign in to comment.