6

I have a long-running child process to which I need to read and write a lot of data. I have a reader thread and a writer thread that manipulate the child.stdout and child.stdin respectively:

extern crate scoped_threadpool;

fn main() {
    // run the subprocess
    let mut child = std::process::Command::new("cat")
        .stdin(std::process::Stdio::piped())
        .stdout(std::process::Stdio::piped())
        .spawn()
        .unwrap();

    let child_stdout = child.stdout.as_mut().unwrap();
    let child_stdin = std::sync::Mutex::new(child.stdin.as_mut().unwrap());

    let mut pool = scoped_threadpool::Pool::new(2);
    pool.scoped(|scope| {
        // read all output from the subprocess
        scope.execute(move || {
            use std::io::BufRead;
            let reader = std::io::BufReader::new(child_stdout);
            for line in reader.lines() {
                println!("{}", line.unwrap());
            }
        });

        // write to the subprocess
        scope.execute(move || {
            for a in 0..1000 {
                use std::io::Write;
                writeln!(&mut child_stdin.lock().unwrap(), "{}", a).unwrap();
            } // close child_stdin???
        });
    });
}

When the writer is done, I want to close child_stdin so that the subprocess finishes and exits, so that the reader sees EOF and pool.scoped returns. I can't do this without child.wait() and I can't call child.wait() because it's being borrowed by the two threads.

How do I make this program complete?

Shepmaster
  • 388,571
  • 95
  • 1,107
  • 1,366
njaard
  • 529
  • 4
  • 15
  • If you are writing a fixed string to the child process, you might want to consider the [subprocess](https://crates.io/crates/subprocess) crate, which implements a [`communicate`](https://docs.rs/subprocess/0.1.11/subprocess/struct.Popen.html#method.communicate) method for what you are now doing with threads. It also exposes a builder-style API which would express the above as `let input = (0..1000).map(|i| format!("{}", i)).collect::(); let output = Exec::cmd("cat").stdin(input).stdout(Redirection::Pipe).capt‌ure()?.stdout_str();` Disclaimer: I am the author of subprocess. – user4815162342 Oct 22 '17 at 15:59

1 Answers1

4

Amusingly, you've caused this yourself by sharing ownership using the Mutex ^_^. Instead of taking a reference to child.stdin, take complete ownership of it and pass it to the thread. When the thread is over, it will be dropped, closing it implicitly:

let mut child_stdin = child.stdin.unwrap();

// ...

scope.execute(move ||
    for a in 0..1000 {
        use std::io::Write;
        writeln!(&mut child_stdin, "{}", a).unwrap();
    }
    // child_stdin has been moved into this closure and is now
    // dropped, closing it.
);

If you'd like to still be able to call wait to get the ExitStatus, change the reference to stdout and the transfer of ownership of stdin to use Option::take. This means that child is not borrowed at all:

let mut child = // ...

let child_stdout = child.stdout.as_mut().unwrap();
let mut child_stdin = child.stdin.take().unwrap();

// ...

child.wait().unwrap();
Shepmaster
  • 388,571
  • 95
  • 1,107
  • 1,366
  • Yes indeed, this works. It's unfortunate that now I can't call `child.wait()`, but I don't need to for my specific program. – njaard Oct 22 '17 at 14:05
  • 2
    @njaard where did you *want* to call `child.wait()`? – Shepmaster Oct 22 '17 at 14:12
  • 1
    Hypothetically, I'd want to get the `ExitStatus` for the child process after it exits (outside of the `scope`) – njaard Oct 22 '17 at 14:35
  • What if from inside the `scope` I want to spawn two threads that both use the stdin object, locked by a mutex? – njaard Oct 23 '17 at 10:16
  • I decided to Think With Lifetimes® and realized I needed an `Arc` for this usecase, because both `stdin` threads need to complete before the `ChildStdin` can be dropped. – njaard Oct 23 '17 at 10:37
  • I would also like to point out to anyone still having issues that you should also make sure to drop the child's stdout or pipe it to null before waiting on it. I just wasted a bunch of time on that. – CoolOppo Oct 13 '19 at 07:36