oconnor663 / shared_child.rs

a wrapper around std::process::Child that lets multiple threads wait or kill at once
MIT License
39 stars 7 forks source link

Allow to construct from std::process::Child #20

Closed ybyygu closed 3 years ago

ybyygu commented 3 years ago

This PR allow me to interact with a long running process's stdin and stdout, more than one time, without deadlock. This is also a new solution to solve #18. I found using os_pipe will get into deadlock in my case.

let child = command
        .new_process_group()
        .stdin(Stdio::piped())
        .stdout(Stdio::piped())
        .spawn()?;
let stdout = child.stdout.take().unwrap();
let stdin = child.stdin.take().unwrap();
let handler = SharedChild::new(child);
# write stdin here
# read result from stdout

sample uses in my repo: https://github.com/ybyygu/vasp-tools/blob/bf0d1c8e79c88b8ba3f47f2029ee2e55a736a3c1/src/session.rs#L356

oconnor663 commented 3 years ago

There's a safety issue here. If the caller gives us a child that's already been waited on, then the system calls we make in this library are invalid. We might end up waiting on some process other than the one we intended.

Maybe we could make broader changes, for example to defensively check the child with a try_wait during construction? But I haven't made any changes to this code in years, so I need to page it all back in and think more about it. What do you think?

ybyygu commented 3 years ago

There's a safety issue here. If the caller gives us a child that's already been waited on, then the system calls we make in this library are invalid. We might end up waiting on some process other than the one we intended.

Thank you for your insightful reply. This may lead to a bug difficult to find. Is it enough to warn about the incorrect usage in documentation?

oconnor663 commented 3 years ago

Coming back to this, I'm starting to think that doing a try_wait check on the child is the right move here. I'm also thinking about adding take_stdin/take_stdout/take_stderr methods similar to what was originally proposed in https://github.com/oconnor663/shared_child.rs/pull/11.

Separate from that, could you tell me what you meant by "interact with a long running process's stdin and stdout, more than one time, without deadlock"? I'm worried that what you're describing here isn't actually possible, but I'm not sure.

ybyygu commented 3 years ago

Separate from that, could you tell me what you meant by "interact with a long running process's stdin and stdout, more than one time, without deadlock"? I'm worried that what you're describing here isn't actually possible, but I'm not sure.

It is like scripting using linux expect command to automate ftp task. This PR works pretty well in my case (interact with VASP command).

https://linux.die.net/man/1/expect