eio
eio copied to clipboard
Add subprocess support
This PR is primarily for generating a discussion around how best to support spawning subprocesses in Eio. In particular for issue #126. The current implementation makes the following decisions.
- Child I/O by default is mapped to
/dev/null - There's a lot of file-descriptor mangling related to child I/O which definitely needs double-checked.
One unfortunate side-effect is that adding subprocesses bypasses all capability-style security iiuc as someone could easily cd .. out of the current working directory of the process. Perhaps processes should live in a separate package?
Note: if it ever lands, it would probably be good to use io-uring-spawn: https://www.phoronix.com/news/Linux-LPC2022-io_uring_spawn
I think processes shouldn't live in another package :-). They're a cornerstone of any unix-y IO library, and having them tightly integrated with Flow is really nice!
If you worry about capability security, maybe this capability should be easy to disable (in an irreversible way)? But I personally think capabilities in OCaml are more about avoiding mistakes, than true security, since there's basically no sandboxing possible.
Thanks @anmonteiro, all seemed sensible and I added the changes in https://github.com/ocaml-multicore/eio/pull/330/commits/b02b37b3a96429b1e5d920ebc1d67803e8a31892
I think I've addressed most comments there is just the relevant C code for being able to change directories for processes in the presence of multiple domains, I'll look into this soon.
One way to make this PR more manageable might be to first split out a PR adding subprocess support to Eio_linux.Low_level (or to eio-luv), then add the cross-platform support in a later PR.
Indeed, that seems like a better approach. Thanks for all the help and reviews so far, I'll open a new linux/luv low-level PR soon!
(For later, getting information about the time spend by the process (user-time, wall-clock) is also an important and difficult information to get in a cross-platform way.)
Closing in favour of #473