non-Unix cfg(unix) ExitStatus is broken

[ijackson]

(Terminology: in this ticket "ExitCode" and "exit status" mean a value like one passed to exit. "ExitStatus" and "wait status" mean a value like one returned as the status value from wait.)

Rust supports a number of platforms which are not Unix but which are #[cfg(unix)] and provide std::process via library/std/src/sys/unix/process/process_unsupported.rs

In rust-lang/libs-team#158 and #106425 (comment) we seem to have decided that we would like to impl From<ExitCode> for ExitStatus. This makes semantic sense. A wait status ought to be able to represent any kind of process completion including an exit status.

Most of process_unsupported is an implementation of imp::ExitStatus (which is used to provide actual public process::ExitStatus). But it has some strange properties which are both unreasonable, and blocking for that From impl.

Properties that ExitStatus actually has right now on these non-unix unix platforms

ExitStatusExt::from_raw and into_raw appear to allow conversion back and forth to and from c_int. (This is implied by providing ExitStatusExt.)

But ExitStatus::into_raw always returns 0 - even for from_raw(nonzero). So the raw conversions are unfaithful.

ExitStatus::code always returns None, even for from_raw(0).

In Nightly, you can call ExitStatus::exit_ok and get an ExitStatusError. Then ExitStatusError::code will give you Option<ExitCode>. This will always return None regardless of the input values.

Properties that ExitStatus ought to have

Given that ExitStatus (the wait status) is inhabited everywhere, even when you can't actually launch and reap a process, and so is ExitCode (the exit status), you should be able to make an ExitStatus (wait status) representing a hypothetical process exit, at the very least.

Certainly unix::ExitStatusExt::from_raw(0).is_success() ought to be true; there are no unix-like platforms where a zero wait status means anything other than successful termination, and on Unix successful process termination is represented as a zero exit status. So we need to be able to represent an 8-bit exit status. ExitStatus::code() should be capable of returning Some.

ExitStatus::from_raw(v).into_raw() should always return v.

Proposal

Given that we have raw conversions we must define what those integers mean.

These non-Unix platforms are providing this API, with stubbed-out implementations, to make it easier to port software. That suggests that we should provide an emulation. Therefore:

process_unsupported::ExitStatus ought to contain a value in "traditional unix exit status wait status" format. This format isn't formally standardised, but roughly speaking it's exit_status << 8, or signal | (core_dumped ? 128 : 0). The methods on ExitStatus in process_unspported.rs ought to decode that trad format and return non-"null" values as appropriate.

Stability

This would be a behavioural change for all the (stable) accessors in ExitStatusExt, but only on platforms using process_unsupported.rs. None of those platforms are actually unix, by definition. They're just pretending (in some cases, for good reasons).

The behavioural change is only visible if ExitStatus::from_raw or ExitStatus as Default is used.

ExitStatus as Default is not in tree yet, but it is being merged in #106425 (FCP completed there). The documentation I write in the followup #114588 is lies on these broken platforms: ExitStatus::default().success() returns false there. I think this is a bug.

from_raw is longstanding. One might argue that none of these changes can be observed by a non-buggy program, since from_raw says "from the raw underlying integer status value from wait" and on these platforms there can be no such values since there is no wait. This is surely too much sophistry. However, in practice, it seems unlikely that many programs would be broken in practice by changing ExitStatus::from_raw(0).code() to return Some(SUCCESS), .success() to return true, etc.;
and it seems unlikely that there are many programs at all using ExitStatus::from_raw(v) where v != 0.

It would make sense to make all these behavioural changes all at once, so provide nontrivial implementations of signal, core_dumped, stopped_signal and continued, as well as code.

Alternatives

Declare that the raw value is precisely the exit status, not shifted left. Or declare that it's the exit status shifted, but don't support the other kinds of value.

[the8472]

ExitStatusExt::from_raw and into_raw appear to allow conversion back and forth to and from c_int. (This is implied by providing ExitStatusExt.)

Ugh. I think this has been a mistake in the first place. It makes it more difficult to use waitid instead of waidpid because the former returns a siginfo_t instead of a status c_int.

I'm currently working waitid so we can use pidfds for race-free waiting. Synthesizing a status value will be necessary there too for into_raw

[ijackson]

I'm currently working waitid so we can use pidfds for race-free waiting. Synthesizing a status value will be necessary there too for into_raw

Yes, I think so. However, I don't think we can get rid of ExitStatusExt::into_raw, nor can we make it not work as expected on actually-Unix platforms. IWBNI libc provided a way to turn si_status and si_code into a wait status, but I don't think synthesising one is going to be too hard.

Anyway I don't think that is directly relevant to this MR ticket, nor the MR #115108 which is trying to clear the way for impl From<ExitCode> for ExitStatus.