Add _timeout methods for cmd to execute

[Binlogo]

Follow suggestion in #91 (comment)

At the same time, adding a timeout/deadline method, while preserving blocking API, would be great. I think it's OK to burn a thread just for that:

• When we spawn a timeout-enabled process, we also spawn a timeout thread which also gets a reference to the Child's handle
• That thread blocks in https://doc.rust-lang.org/stable/std/sync/mpsc/struct.Receiver.html#method.recv_timeout
• if timeout elapses, it kills the process, which unblocks the main thread.
• otherwise, it unblocks when the sender side of the channel is blocked (it's important that we don't leave hanging threads around)

Add timeout relative methods: run_timeout / read_timeout / read_stderr_timeout / output_timeout.

[matklad]

Rather than adding a separate xxx_timeout method for each of operations we have, let's rather add a single .timeout() builder method, a-la ignore_status

[Binlogo]

Nice advice, it will be much neat.

[matklad]

This is not quiet right --- we are "leaking" a thread handle here. As we are not manually .joining the thread, this means this thread can continue running past the point where output_impl returns, which violates structured concurrency.

We should enforce the invariant that, by the time output_impl returns, the child thread is .joined.

We should also proactively .kill the child process when the timeout happens. My understanding is that the current behavior is that the function retuns with an error after the timeout elapses, but the actual child process continues to execute, which is not what we want here.

[Binlogo]

Yes, you're absolutely right. Thank you for pointing it out. I'll find an appropriate place to .join and .kill this thread to prevent the "leaking".

[Binlogo]

However, it seems that I hit the wall:

        let out_res = if let Some(timeout) = self.data.timeout {
            let (tx, rx) = mpsc::channel();
            let handle = thread::spawn(move || {
                let output = child.wait_with_output();
                let _ = tx.send(output);
            });
            handle.join().unwrap();
            match rx.recv_timeout(timeout) {
                Ok(output) => output,
                Err(err) => {
                    // FIXME: Kill the child, borrow of moved value: `child`
                    // child.kill();
                    return Err(Error::new_timeout(self, err));
                }
            }
        } else {
            child.wait_with_output()
        };

Since .wait_with_output need the child's ownership, when spawned, I can't use it to call .kill anymore. I tried Arc way, but it still failed:

error[E0507]: cannot move out of dereference of MutexGuard<'_, Child>

@matklad Do you have any suggestion for this situation?

[matklad]

Huh, that's nasty! This sadly might require us to essentially re-implement wait_with_output, at least for the case where there's a timeout

[dylni]

@Binlogo Saw this issue and thought I would mention this is basically the entire purpose of process_control.

[Binlogo]

thanks for your information, it seems fit this situation, i'll take a look.

[matklad]

sleep here is an utility from the OS, which is OS-specific and might not be present. For tests, we would want to use our own version of sleep. See how the xecho is implemented and used for similar tests, and use the same pattern for xsleep.