oconnor663 / duct.rs

a Rust library for running child processes
MIT License
795 stars 34 forks source link

Add support for Windows creation_flags #70

Open lzybkr opened 5 years ago

lzybkr commented 5 years ago

I need to use std::os::windows::process::CommandExt::creation_flags when starting a process.

It looks like I could use before_spawn, but it would be nice to have a more direct api.

oconnor663 commented 5 years ago

This is pretty much exactly the sort of thing that before_spawn was intended to support. My theory was that by the time people are reaching for complex process-spawning flags and platforms-specific APIs, they're pretty close to needing to ditch duct entirely and control all their own pipes and process spawning. (For example, say you need special flags for opening the pipes themselves. That'll probably never work in duct, and I'd be surprised if even std::process::Command is flexible enough for your needs.) So it probably doesn't make sense to expand the duct API too much for those edge cases. Let me know what you think.

lzybkr commented 5 years ago

It's probably not a big deal either way, but my thoughts before opening the issue:

I haven't started using duct, but I'll be trying it soon for my scenario. I'm implementing a debugger and need to pass DEBUG_PROCESS when starting the process. It works great right now, and I'm capturing stdout/stderr in different pipes - so I was hoping duct would help merge the pipes easily. It seems like it will.

After reviewing the creation flags - I do think many would work just fine with duct.

oconnor663 commented 5 years ago

Gotcha. Please let me know if the before_spawn approach gives you any trouble.

Boscop commented 3 years ago

(Sorry to hijack this old conversation.) I'm doing this:

// prevent child processes from being terminated by ctrl-c
cfg_if! {
    if #[cfg(windows)] {
        pub fn prevent_being_killed(cmd: &mut Command) -> std::io::Result<()> {
            cmd.creation_flags(CREATE_NEW_PROCESS_GROUP);
            Ok(())
        }
    } else {
        pub fn prevent_being_killed(_cmd: &mut Command) -> std::io::Result<()> {
            // what to do on linux/mac?
            Ok(())
        }
    }
}

I'm using it with .before_spawn(prevent_being_killed) to put child ffmpeg processes in a new process group, so that I can endure a graceful termination and let ffmpeg finalize the video file when receiving Ctrl-C. It works, but only on windows. @oconnor663 Do you know what the linux equivalent of this is? I only found this, it requires nohup and I don't know what the Rust equivalent of passing preexec_fn=os.setpgrp is. I'd really appreciate if you could help me out here :)

oconnor663 commented 3 years ago

@Boscop something like this using libc seems to work for me on Linux. Is this in the ballpark of what you're looking for?

use std::os::unix::process::CommandExt;

fn main() {
    unsafe {
        std::process::Command::new("sleep")
            .arg("100")
            .pre_exec(|| {
                libc::setpgid(0, 0); // technically this can fail and I'm not checking
                Ok(())
            })
            .status()
            .unwrap();
    }
}

When I Ctrl-C that program, I see that sleep is still running, which I think is the goal. Note that I've swapped setpgrp for setpgid, since it looks like the former might be less portable. If you wanted to accomplish the same while working with duct, you'd use Expression::before_spawn() to get ahold of the &mut Command, and then call pre_exec as here. (I've left out the nohup part, but we could add that in, or maybe suppress the signal in the child?)

Boscop commented 3 years ago

Thanks! (Btw, it's weird that setpgid doesn't show up in the docs search: https://docs.rs/libc/0.2.91/libc/?search=setpgid)

I now wrote it like this to handle errors:

pub fn prevent_being_killed(cmd: &mut Command) -> std::io::Result<()> {
    use nix::{Error, unistd::{Pid, setpgid}};
    use std::{io, os::unix::process::CommandExt};

    unsafe {
        cmd.pre_exec(|| {
            match setpgid(Pid::from_raw(0), Pid::from_raw(0)) {
                Ok(()) => Ok(()),
                Err(Error::Sys(errno)) => Err(io::Error::from_raw_os_error(errno as _)),
                Err(e) => Err(
                    io::Error::new(io::ErrorKind::Other,
                        format!("failed to create new process group for child process: {}", e)
                    )
                ),
            }
        });
    }
    Ok(())
}

Btw, how could I intercept SIGHUP in the parent and ignore it in the child process? (I'm using the ctrlc crate to intercept SIGINT and SIGTERM in the parent.)

oconnor663 commented 3 years ago

@Boscop suppressing a signal in the child is relatively simple. You use the POSIX signal function with the SIG_IGN ("signal ignore") disposition. In nix terms, I think that should be nix::sys::signal::signal together with nix::sys::signal::SigHandler::SigIgn. (Unfortunately it looks like that's unsafe for...reasons...but ignoring a signal is generally a perfectly safe thing to do, so hopefully you're fine here.)

Handling the signal in the parent is substantially more involved. I think the signal-hook crate is currently the way to go here. It has a wide variety of options to choose from. On the other hand, if you're writing a Tokio app, it might make more sense to reach for the tokio::signal module. (I'm sure integrating Tokio and signal-hook is possible, but it might require a dedicated thread or something like that.)

ArtemGr commented 2 years ago

There's also https://man7.org/linux/man-pages/man2/setsid.2.html.