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.

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

std::os::pipe doesn't handle error correctly #14724

Closed
hannobraun opened this issue Jun 7, 2014 · 0 comments · Fixed by #14781
Closed

std::os::pipe doesn't handle error correctly #14724

hannobraun opened this issue Jun 7, 2014 · 0 comments · Fixed by #14781

Comments

@hannobraun
Copy link

The function std::os::pipe just fails if the call to libc::pipe returns an error code.

This can be easily triggered with the following program:

use std::io::Command;

fn main() {
    let mut p = Vec::new();

    loop {
        match Command::new("cat").spawn() {
            Ok(process) =>
                p.push(process),
            Err(error) =>
                fail!("Error: {}", error)
        }

        print!("{}\n", p.len());
    }
}

On my machine (Ubuntu 14.04, x86_64), the program fails at 338 processes with the following message:

task '<main>' failed at 'assertion failed: `(left == right) && (right == left)` (left: `-1`, right: `0`)', /home/hanno/Projects/vndf/vendor/rust/src/libstd/os.rs:517

I believe that os::pipe() should return a Result (IoResult?) which should be passed up the chain. When I tried implementing this change, however, I quickly ran into trouble with users of the function. While some already return IoResult and can just pass the error up the chain, others don't. I wasn't sure, if I should change even more function signatures or just fail in these cases.

Since this isn't a problem for me currently and my time is very limited, I won't work further on this, sorry!
Maybe someone who understands the requirements of the calling code better might want to take a look.

alexcrichton added a commit that referenced this issue Jun 16, 2014
* os::pipe() now returns IoResult<os::Pipe>
* os::pipe() is now unsafe because it does not arrange for deallocation of file
  descriptors
* os::Pipe fields are renamed from input to reader and out to write.
* PipeStream::pair() has been added. This is a safe method to get a pair of
  pipes.
* Dealing with pipes in native process bindings have been improved to be more
  robust in the face of failure and intermittent errors. This converts a few
  fail!() situations to Err situations.

Closes #9458
cc #13538
Closes #14724
[breaking-change]
bors added a commit that referenced this issue Jun 16, 2014
* os::pipe() now returns `IoResult<os::Pipe>`
* os::pipe() is now unsafe because it does not arrange for deallocation of file
  descriptors
* PipeStream::pair() has been added. This is a safe method to get a pair of
  pipes.
* Dealing with pipes in native process bindings have been improved to be more
  robust in the face of failure and intermittent errors. This converts a few
  fail!() situations to Err situations.

cc #13538
Closes #14724
[breaking-change]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant