Skip to content
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.

Sign up for GitHub

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

Poll loop fixes #11624

Merged
merged 2 commits into from
Mar 20, 2023
Merged

Poll loop fixes #11624

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
493ca33
Fix incorrect comment.
vext01 Jan 25, 2023
5c3825f
Set non-blocking IO more robustly.
vext01 Jan 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 12 additions & 5 deletions crates/cargo-util/src/read2.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,21 +2,28 @@ pub use self::imp::read2;

#[cfg(unix)]
mod imp {
use libc::{c_int, fcntl, F_GETFL, F_SETFL, O_NONBLOCK};
use std::io;
use std::io::prelude::*;
use std::mem;
use std::os::unix::prelude::*;
use std::process::{ChildStderr, ChildStdout};

fn set_nonblock(fd: c_int) -> io::Result<()> {
let flags = unsafe { fcntl(fd, F_GETFL) };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is some uncertainty whether or not F_GETFL takes an argument. https://stackoverflow.com/questions/25061656/when-is-the-arg-for-f-getfl-fcntl-command-required has an interesting discussion of the issue. The Open Group specification notes that the arg value is reserved for future growth, without specifying what that means or what it should be set to or whether or not it should be specified. FreeBSD specifically says it is ignored. Very strange.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On OpenBSD and Linux the manual also says the argument is ignored.

if flags == -1 || unsafe { fcntl(fd, F_SETFL, flags | O_NONBLOCK) } == -1 {
return Err(io::Error::last_os_error());
}
Ok(())
}

pub fn read2(
mut out_pipe: ChildStdout,
mut err_pipe: ChildStderr,
data: &mut dyn FnMut(bool, &mut Vec<u8>, bool),
) -> io::Result<()> {
unsafe {
libc::fcntl(out_pipe.as_raw_fd(), libc::F_SETFL, libc::O_NONBLOCK);
libc::fcntl(err_pipe.as_raw_fd(), libc::F_SETFL, libc::O_NONBLOCK);
}
set_nonblock(out_pipe.as_raw_fd())?;
set_nonblock(err_pipe.as_raw_fd())?;

let mut out_done = false;
let mut err_done = false;
Expand All @@ -32,7 +39,7 @@ mod imp {
let mut errfd = 1;

while nfds > 0 {
// wait for either pipe to become readable using `select`
// wait for either pipe to become readable using `poll`
let r = unsafe { libc::poll(fds.as_mut_ptr(), nfds, -1) };
if r == -1 {
let err = io::Error::last_os_error();
Expand Down