Add setsid method to CommandExt trait

[HarveyHunt]

Proposal

Problem statement

Currently, if you want to use Command to spawn a new process and create a new session, then you need to use the pre_exec method. However, calling pre_exec means that the POSIX spawn fast path can't be used which makes spawning a new process more expensive than necessary.

By exposing the ability to pass the POSIX_SPAWN_SETSID flag to the POSIX spawn method it becomes possible to use the fast path to create processes. It also removes the need for unsafe code.

Motivation, use-cases

I don't have example code to share here, but I've been developing and maintaining a system that spawns multiple processes a second. These processes are started in new sessions to allow the main process to handle graceful shutdown upon receiving CTRL+C from the terminal.

Using the existing pre_exec method, the fork + exec required uses considerable kernel CPU. Switching to using the POSIX spawn fast path reduced the kernel CPU usage by ~60% IIRC.

Solution sketches

The std::os::unix::process::CommandExt trait can be updated to include a new setsid method, with the following signature:

trait CommandExt {
    fn setsid(&mut self) -> &mut Command
}

If a user calls .setsid() then the POSIX_SPAWN_SETSID flag will be passed to POSIX spawn.

A slightly different interface could have the following signature:

trait CommandExt {
    fn setsid(&mut self, setsid: bool) -> &mut Command
}

This would allow the caller to call setsid(false) to remove the setsid option from a Command that previously had it set. However, I'm not sure how useful this is in practice and it makes the method more verbose to use.

Links and related work

In my excitement to add this, I've done things in the wrong order and have submitted a PR and tracking issue already.

Rust PR: rust-lang/rust#105377
Rust Tracking issue: rust-lang/rust#105376

Man page for posix_spawn: https://man7.org/linux/man-pages/man3/posix_spawn.3.html

What happens now?

This issue is part of the libs-api team API change proposal process. Once this issue is filed the libs-api team will review open proposals in its weekly meeting. You should receive feedback within a week or two.

[cuviper]

What if it is not supported? Is it possible to have a fallback? e.g. python issue 37586 discusses a problem with support added in macOS 10.15, where apparently older versions would just ignore the unknown flag. (AFAICS the libc crate has not yet added a macOS definition for this at all.)

For Linux, my manpage says glibc added this flag in 2.26 (but our minimum is 2.17), and musl WHATSNEW says they added it in 1.1.17. I don't see support in uClibc at all.

[pitaj]

One option would be to return Result<Command, Command>. Returns Ok(self) if setsid is implemented on the platform, or Err(self) if it is not.

Another option would be to provide a more generic set_flags method along with a SETSID constant that's documented to not always work. Or just not add the constant and allow the user to pass in flags themselves or from libc.

But if it's okay to just ignore when it's not supported, I think that's probably the best option. As long as it's documented well.

[thomcc]

Is setsid unsupported on any platforms, or just the posix_spawn flag? Certainly on macOS setsid is supported further back, it's just the posix_spawn flag that isn't available in all versions (we'd have to use version checking to detect this on macOS, but that isn't a significant burden).

This means we could opportunistically implement it with fork/exec where the posix_spawn flag isn't available — the posix_spawn use would just be an optimization on top of that. (We already have some cases like this in the code)

[cuviper]

Yeah, setsid seems plenty standard (POSIX.1-2001, POSIX.1-2008, SVr4), and libc has it defined unconditionally in src/unix/mod.rs, so we should be ok using it under fork/exec with POSIX_SPAWN_SETSID as an optimization.

[pitaj]

Are there any drawbacks to always using POSIX_SPAWN_SETSID when available or is it purely beneficial?

[HarveyHunt]

Thanks for the comments about backwards compatibility, I hadn't really thought about that so thanks for the input! I think that transparently using POSIX_SPAWN_SETSID if supported is a nice idea - could we determine that at compile time or would we need to check at runtime?

Are there any drawbacks to always using POSIX_SPAWN_SETSID when available or is it purely beneficial?

I'm not aware of any drawbacks, I spent a bit of time looking at the history of POSIX_SPAWN_SETSID becoming available, but can't find any mention of drawbacks or tradeoffs. I'd guess that the main issue is about availability of the flag, as we've been discussing.

Here's the glibc commit that added support for it: https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=daeb1fa2e1b33323e719015f5f546988bd4cc73b

The Austin Group discussion about adding it to POSIX is here: https://www.austingroupbugs.net/view.php?id=1044

[pitaj]

I feel like it should be possible to do at compile time.

[cuviper]

We compile std with older system minimums -- glibc 2.17, macOS 10.7, etc. -- so we need runtime checks to see what's actually available. And as I mentioned earlier from the Python issue, we apparently can't even rely on EINVAL from posix_spawnattr_setflags when it is not supported.

[pitaj]

We compile std with older system minimums -- glibc 2.17, macOS 10.7, etc. -- so we need runtime checks to see what's actually available.

Ah, good point. At best, it would only be possible at compile time if using -Zbuild-std.

And as I mentioned earlier from the Python issue, we apparently can't even rely on EINVAL from posix_spawnattr_setflags when it is not supported.

Does that matter though? If we're just using it opportunistically and make no guarantees, can't we just always pass the flag when it won't cause an error?

[cuviper]

Does that matter though? If we're just using it opportunistically and make no guarantees, can't we just always pass the flag when it won't cause an error?

It matters because it's a silent failure -- we won't know that we need to use the fork/exec fallback to call setsid(). Maybe that's what you mean by "make no guarantees", but it's not useful if we don't actually do it.

[the8472]

We could exclude macos from the posix_spawn path until we raise the minimum to 10.15?

[pitaj]

@cuviper Ah yeah makes sense. I misunderstood some context. I agree with the plan to have setsid use the flag if the flag can be guaranteed to work, otherwise fall back to pre_exec.

We could exclude macos from the posix_spawn path until we raise the minimum to 10.15?

That would be unfortunate. Can we detect if we're on a supported OS version? Maybe perform the check once in the pre-main init code?

[thomcc]

Yes, the check of macOS version would be easy (although we don't have anywhere that does it already). We shouldn't do it in pre-main because Rust may not be in control of main, but that doesn't really matter, I would not really be in favor of doing it there anyway.

[pitaj]

We shouldn't do it in pre-main because Rust may not be in control of main, but that doesn't really matter, I would not really be in favor of doing it there anyway.

Good point. Any idea how expensive the version check is? Would it need to be memoized?

[thomcc]

It's pretty cheap. Apple doesn't memoize the calls to the function that @available gets lowered to in Swift or objective C. It would be easy to change that if it turns out to be worth it.

[HarveyHunt]

Thanks for the discussion here, sorry it's taken me some time to respond.

It looks like the next step is to implement logic to check whether setsid is supported by POSIX spawn at run time. I can start working on that and update the linked PR with the changes.

Are there any good examples or pointers to how this is done in other parts of the stdlib? Thanks!

[the8472]

We discussed this during today's libs-api meeting. We're on board with it especially due to the performance savings from posix_spawn which would not otherwise be available to user code.