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

Add methods to get access to child's io streams #11

Closed faern closed 7 years ago

faern commented 7 years ago

I realized one thing not possible here that was possible in clonablechild is to get access to the io streams of the child.

oconnor663 commented 7 years ago

I've only thought a little bit about the IO handles, and I'm not sure quite what I'd like to do. I wonder if it would make sense to have the same fields directly on the SharedChild, and populate them during spawn? But that would interact in a weird way with into_inner; the child you get back would be missing its streams, even if you didn't use them. (I wish they implemented try_clone on those types.) It would be really cool if we could expose reading and writing through shared references (as opposed to the first-writer-wins situation we get with Mutex+Option::take), very much in the spirit of the rest of the crate. But unfortunately ChildStdout and the others don't implement Read/Write with shared references, even though File does.

I have this other crate, os_pipe, which lets you create double-ended pipes of your own that are compatible with std::process. Those do support shared reading and writing, and also try_clone. (They're secretly just a File on the inside). In my head, I've kind of imagined that anyone who wants to do fancy things with IO will just create their own pipes anyway. There's also the duct library, which is built on top of these two, and which should be much more convenient for the common use cases.

What do you think?

faern commented 7 years ago

My goal with clonablechild was to expose the same functionality as the standard Child, only difference that it would be possible to wait for it and kill it in different threads. Simple as that.

I don't really need advanced piping or anything. I just want to run one child process that I'm able to instantly detect if it dies (blocking on wait in a thread) and at the same time read the stdout/stderr in other threads. Also being able to kill the child at will from some other thread.

Maybe I should start looking at the subprocess crate or something. I was just hoping to do it with bare minimum. But not being able to read the stdout is a blocker.

faern commented 7 years ago

I have a long running subprocess that I need to instantly restart if it dies, but I also want to be able to kill it if the user decides to quit my program. While it is running I want to read the stdout or be able to redirect it to file or something.

oconnor663 commented 7 years ago

Yeah I'm curious whether duct would work well for you, particularly the .stdout(&str) file redirection and the .read() output capturing. (If you want streaming output though, you'd need to pass in os_pipe pipes to .stdout_handle() etc.)

oconnor663 commented 7 years ago

Getting back to this one, I think I'd like to do one of two things:

  1. Just tell people to use os_pipe if they want this functionality. The into_stdio method in that crate is designed to let you pass pipes (and other things) to the standard library's Command. Then you have complete control over both ends of the pipe, and you can do whatever you want, including try_clone. (Unfortunately you have to let the Command drop before you can read EOF from the pipe, because it's holding the write end open.)
  2. Expose types from os_pipe directly. We could get our hands on the Child's file descriptors with as_raw_fd and then duplicate them, and convert the result into Option<os_pipe::PipeReader>. Then you could even read from an Arc<SharedChild>, because PipeReader (like File but unlike ChildStdout) implements Read with a shared reference. [Note to self, I should write a PR adding those impls.]

By the way, thanks a million for https://github.com/oconnor663/shared_child.rs/pull/13. I'm going to add you to this repo as a committer, though I'd like to keep going through the PR process for anything other than tiny changes, if that's ok with you.

faern commented 7 years ago

Awesome. Thanks for adding me as a committer! Of course I'll submit PRs for you anyway. You still know a lot more than me how this repo interacts with os_pipe and duct. Plus, it's always good to be multiple people to settle on naming and other stuff.

Regarding this PR and exposing the streams. I just started using duct for my subprocess management yesterday. I have not come to the stage where I handle the streams yet. But if it lets me chose between saving the output to a file and getting a stream directly in my parent in some way I'm satisfied.

faern commented 7 years ago

I think this PR is pretty irrelevant by now.

oconnor663 commented 7 years ago

Hey @faern, looking back on this after a long break (sorry about that), I think it might make sense to land it more or less as-is. What makes you say it's irrelevant?