Skip to content

non-Unix cfg(unix) ExitStatus is broken #114593

Closed

Description

(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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Assignees

No one assigned

    Labels

    C-bugCategory: This is a bug.T-libsRelevant to the library team, which will review and decide on the PR/issue.T-libs-apiRelevant to the library API team, which will review and decide on the PR/issue.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions