duct.rs icon indicating copy to clipboard operation
duct.rs copied to clipboard

Setup and kill process group instead of just immediate child

Open passcod opened this issue 8 years ago • 6 comments

See how watchexec does it: https://github.com/mattgreen/watchexec/blob/master/src/process.rs

The stdlib Process sends a kill syscall to the process: https://github.com/rust-lang/rust/blob/master/src/libstd/sys/unix/process/process_unix.rs#L246

However, that doesn't kill all processes, especially with servers, see passcod/cargo-watch#25.

I really don't want to implement process handling myself. That's what Duct does. So it would be really neat if Duct could handle it :)

passcod avatar Mar 31 '17 02:03 passcod

I thought about using process groups when I was figuring out how to implement kill safely in https://github.com/oconnor663/shared_child.rs. The major downside with process groups though, is that if you have children that aren't in the same group as the parent, then Ctrl-C in the terminal won't kill them when it kills the parent. The solution there is to have the parent handle SIGINT and kill the children group itself before exiting, and it looks like watchexec does something like that. But duct is just a library, and it doesn't own the process-wide signal handlers, so we can't do that. I don't know of any other solutions.

Using process groups to guarantee cleanup might run into a related issue, where if any of the grandchild processes change their group, they won't receive signals anymore. That might happen if watchexec executes another instance of watchexec, for example, maybe via some script that runs watchexec under the covers. I think really solving that would require using some platform-specific features like cgroups. Or Docker, which is based on cgroups. And speaking of Docker, even cgroup-based cleanup can be incomplete if there's any service running on the machine that will spawn child processes in response to commands, like dockerd. At that point the OS has no idea who's really responsible for those processes, and all bets are off :p

I worry that if duct did expose process groups, the documentation for how it all works would be full of warnings and limitations, since different use cases have such different requirements (trusted children vs untrusted, interactive commands vs services, Rust programs vs libraries linked from other environments). My assumption has mostly been that, if you're a program whose primary job is to manage child processes, you'll want to get your hands on all the low level functions yourself rather than letting duct hide them from you. But I could be convinced otherwise, if there were a few different callers that had mostly compatible requirements?

oconnor663 avatar Mar 31 '17 15:03 oconnor663

Hmm, maybe just having a way to expose the process ID of the running expression, so Duct-users can still get its abstractions but also call OS functions directly?

I'm actually fine with figuring out low-level for specialised usecases, but it seems so… wasteful, at a personal and at a community level, to do all of it over and over and over again, when 80% of what I need is covered by Duct.

passcod avatar Mar 31 '17 23:03 passcod

One thing I've been thinking about is a constructor that lets you build an Expression from a std::process::Command object that you've prepared yourself. That would let you get at the Unix-specific CommandExt trait functions as needed, without requiring duct to have much of an opinion about what's exposed and what isn't. Would that work for you?

Exposing the SharedChild objects from inside a Handle could be possible too. Tell me more about how you might want to use them?

oconnor663 avatar Mar 31 '17 23:03 oconnor663

That… could work, yes. Especially combined with an externalised sh from the other issue, so I could build commands whether they be straight calls or shell calls, attach the control I want, and then pass them into Duct.

passcod avatar Apr 01 '17 00:04 passcod

Having the PID of a command would mean, I believe, I could create a process group, and then assign commands to the group. With setpgid, I can assign a pid to a pgid. On Windows, I can use AssignProcessToJobObject to do the same. I'd need to create the process groups / job objects myself prior to running the commands, but that's probably fine.

passcod avatar Apr 01 '17 00:04 passcod

Thinking out loud: Giving you the PIDs is going to be tricky, because of then. The right side of a then expression doesn't execute until the left side is finished. A method like Handle::get_all_pids() wouldn't be able to give you a complete list. And if you did any kind of out-of-band signaling that duct didn't know about, it could produce weird results. (Duct knows that if you kill a running expression, the right side of a then should not be started after that, even if the left side is unchecked or happened to have just exited. But if you did a group kill, the right side of then could still execute.) Can we solve these problems?

oconnor663 avatar Apr 02 '17 18:04 oconnor663