| Age | Commit message (Collapse) | Author | Lines |
|---|
|
Add From<u8> for ExitCode
This should cover a mostly cross-platform subset of supported exit codes.
We decided to stick with `u8` initially since its the common subset between all platforms that we support (excluding wasm which I think only works with `true` or `false`). Posix is supposed to take i32s, but in practice many unix platforms mask out all but the low 8 bits or in some cases the 8-15th bits. Windows takes a u32 instead of an i32. Bourne-compatible shells also report signals as exitcode 128 + `signal_no`, so there's some ambiguity there when returning exit codes > 127, but it is possible to disambiguate them on the other side so we decided against restricting the possible codes further than to `u8`.
## Related
- Detailed analysis of exit code support on various platforms: https://internals.rust-lang.org/t/mini-pre-rfc-redesigning-process-exitstatus/5426
- https://github.com/rust-lang/rust/issues/48711
- https://github.com/rust-lang/rust/issues/43301
- https://rust-lang.zulipchat.com/#narrow/stream/219381-t-libs/topic/Termination.2FExit.20Status.20Stabilization
|
|
This should cover a mostly cross-platform subset of supported exit codes.
|
|
|
|
|
|
|
|
This makes a few changes to the weak symbol macros in `sys::unix`:
- `dlsym!` is added to keep the functionality for runtime `dlsym`
lookups, like for `__pthread_get_minstack@GLIBC_PRIVATE` that we don't
want to show up in ELF symbol tables.
- `weak!` now uses `#[linkage = "extern_weak"]` symbols, so its runtime
behavior is just a simple null check. This is also used by `syscall!`.
- On non-ELF targets (macos/ios) where that linkage is not known to
behave, `weak!` is just an alias to `dlsym!` for the old behavior.
- `raw_syscall!` is added to always call `libc::syscall` on linux and
android, for cases like `clone3` that have no known libc wrapper.
The new `weak!` linkage does mean that you'll get versioned symbols if
you build with a newer glibc, like `WEAK DEFAULT UND statx@GLIBC_2.28`.
This might seem problematic, but old non-weak symbols can tie the build
to new versions too, like `dlsym@GLIBC_2.34` from their recent library
unification. If you build with an old glibc like `dist-x86_64-linux`
does, you'll still get unversioned `WEAK DEFAULT UND statx`, which may
be resolved based on the runtime glibc.
I also found a few functions that don't need to be weak anymore:
- Android can directly use `ftruncate64`, `pread64`, and `pwrite64`, as
these were added in API 12, and our baseline is API 14.
- Linux can directly use `splice`, added way back in glibc 2.5 and
similarly old musl. Android only added it in API 21 though.
|
|
Unix ExitStatus comments and a tiny docs fix
Some nits left over from #88300
|
|
With cross-reference.
Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
|
|
Co-authored-by: Josh Triplett <josh@joshtriplett.org>
|
|
|
|
In #89522 we learned that `clone3` is interacting poorly with Gentoo's
`sandbox` tool. We only need that for the unstable pidfd extensions, so
otherwise avoid that and use a normal `fork`.
|
|
The fuchsia platform is in the process of softly transitioning over to
using a new value for ZX_INFO_PROCESS with a new corresponding struct.
This change migrates libstd.
See fxrev.dev/510478 and fxbug.dev/30751 for more detail.
|
|
Use `libc::sigaction()` instead of `sys::signal()` to prevent a deadlock
Fixes #88585. POSIX [specifies](https://man7.org/linux/man-pages/man3/fork.3p.html) that after forking,
> to avoid errors, the child process may only execute async-signal-safe operations until such time as one of the exec functions is called.
Rust's standard library does not currently adhere to this, as evidenced by #88585. The child process calls [`sys::signal()`](https://github.com/rust-lang/rust/blob/7bf0736e130e2203c58654f7353dbf9575e49d5c/library/std/src/sys/unix/android.rs#L76), which on Android calls [`libc::dlsym()`](https://github.com/rust-lang/rust/blob/7bf0736e130e2203c58654f7353dbf9575e49d5c/library/std/src/sys/unix/weak.rs#L101), which is [**not**](https://man7.org/linux/man-pages/man7/signal-safety.7.html) async-signal-safe, and in fact causes a deadlock in the example in #88585.
I think the easiest solution here would be to just call `libc::sigaction()` instead, which [is](https://man7.org/linux/man-pages/man7/signal-safety.7.html) async-signal-safe, provides the functionality we need, and is apparently available on all Android versions because it is also used e.g. [here](https://github.com/rust-lang/rust/blob/7bf0736e130e2203c58654f7353dbf9575e49d5c/library/std/src/sys/unix/stack_overflow.rs#L112-L114).
|
|
Manual Debug for Unix ExitCode ExitStatus ExitStatusError
These structs have misleading names. An ExitStatus[Error] is actually a Unix wait status; an ExitCode is actually an exit status. These misleading names appear in the `Debug` output.
The `Display` impls on Unix have been improved, but the `Debug` impls are still misleading, as reported in #74832.
Fix this by pretending that these internal structs are called `unix_exit_status` and `unix_wait_status` as applicable. (We can't actually rename the structs because of the way that the cross-platform machinery works: the names are cross-platform.)
After this change, this program
```
#![feature(exit_status_error)]
fn main(){
let x = std::process::Command::new("false").status().unwrap();
dbg!(x.exit_ok());
eprintln!("x={:?}",x);
}
```
produces this output
```
[src/main.rs:4] x.exit_ok() = Err(
ExitStatusError(
unix_wait_status(
256,
),
),
)
x=ExitStatus(unix_wait_status(256))
```
Closes #74832
|
|
|
|
The reference automatically coerces to a pointer. Writing an explicit
cast here is slightly misleading because that's most commonly used when
a pointer needs to be converted from one pointer type to another, e.g.
`*const c_void` to `*const sigaction` or vice versa.
|
|
|
|
These structs have misleading names. An ExitStatus[Error] is actually
a Unix wait status; an ExitCode is actually an exit status.
The Display impls are fixed, but the Debug impls are still misleading,
as reported in #74832.
Fix this by pretending that these internal structs are called
`unix_exit_status` and `unix_wait_status` as applicable. (We can't
actually rename the structs because of the way that the cross-platform
machinery works: the names are cross-platform.)
Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
|
|
|
|
|
|
|
|
Introduce `OwnedFd` and `BorrowedFd`, and the `AsFd` trait, and
implementations of `AsFd`, `From<OwnedFd>` and `From<T> for OwnedFd`
for relevant types, along with Windows counterparts for handles and
sockets.
Tracking issue:
- <https://github.com/rust-lang/rust/issues/87074>
RFC:
- <https://github.com/rust-lang/rfcs/blob/master/text/3128-io-safety.md>
|
|
|
|
Add Linux-specific pidfd process extensions (take 2)
Continuation of #77168.
I addressed the following concerns from the original PR:
- make `CommandExt` and `ChildExt` sealed traits
- wrap file descriptors in `PidFd` struct representing ownership over the fd
- add `take_pidfd` to take the fd out of `Child`
- close fd when dropped
Tracking Issue: #82971
|
|
|
|
|
|
Improve docs
Split do_fork into two
Make do_fork unsafe
Add target attribute to create_pidfd field in Command
Add method to get create_pidfd value
|
|
Co-authored-by: bjorn3 <bjorn3@users.noreply.github.com>
|
|
Background:
Over the last year, pidfd support was added to the Linux kernel. This
allows interacting with other processes. In particular, this allows
waiting on a child process with a timeout in a race-free way, bypassing
all of the awful signal-handler tricks that are usually required.
Pidfds can be obtained for a child process (as well as any other
process) via the `pidfd_open` syscall. Unfortunately, this requires
several conditions to hold in order to be race-free (i.e. the pid is not
reused).
Per `man pidfd_open`:
```
· the disposition of SIGCHLD has not been explicitly set to SIG_IGN
(see sigaction(2));
· the SA_NOCLDWAIT flag was not specified while establishing a han‐
dler for SIGCHLD or while setting the disposition of that signal to
SIG_DFL (see sigaction(2)); and
· the zombie process was not reaped elsewhere in the program (e.g.,
either by an asynchronously executed signal handler or by wait(2)
or similar in another thread).
If any of these conditions does not hold, then the child process
(along with a PID file descriptor that refers to it) should instead
be created using clone(2) with the CLONE_PIDFD flag.
```
Sadly, these conditions are impossible to guarantee once any libraries
are used. For example, C code runnng in a different thread could call
`wait()`, which is impossible to detect from Rust code trying to open a
pidfd.
While pid reuse issues should (hopefully) be rare in practice, we can do
better. By passing the `CLONE_PIDFD` flag to `clone()` or `clone3()`, we
can obtain a pidfd for the child process in a guaranteed race-free
manner.
This PR:
This PR adds Linux-specific process extension methods to allow obtaining
pidfds for processes spawned via the standard `Command` API. Other than
being made available to user code, the standard library does not make
use of these pidfds in any way. In particular, the implementation of
`Child::wait` is completely unchanged.
Two Linux-specific helper methods are added: `CommandExt::create_pidfd`
and `ChildExt::pidfd`. These methods are intended to serve as a building
block for libraries to build higher-level abstractions - in particular,
waiting on a process with a timeout.
I've included a basic test, which verifies that pidfds are created iff
the `create_pidfd` method is used. This test is somewhat special - it
should always succeed on systems with the `clone3` system call
available, and always fail on systems without `clone3` available. I'm
not sure how to best ensure this programatically.
This PR relies on the newer `clone3` system call to pass the `CLONE_FD`,
rather than the older `clone` system call. `clone3` was added to Linux
in the same release as pidfds, so this shouldn't unnecessarily limit the
kernel versions that this code supports.
Unresolved questions:
* What should the name of the feature gate be for these newly added
methods?
* Should the `pidfd` method distinguish between an error occurring
and `create_pidfd` not being called?
|
|
`weak!` is needed in a test in another module. With macros
1.0, importing `weak!` would require reordering module
declarations in `std/src/lib.rs`, which is a bit too
evil.
|
|
|
|
Provide ExitStatusError
Closes #73125
In MR #81452 "Add #[must_use] to [...] process::ExitStatus" we concluded that the existing arrangements in are too awkward so adding that `#[must_use]` is blocked on improving the ergonomics.
I wrote a mini-RFC-style discusion of the approach in https://github.com/rust-lang/rust/issues/73125#issuecomment-771092741
|
|
Some platforma (eg ARM64) apparently generate SIGTRAP for panic abort!
See eg
https://github.com/rust-lang/rust/pull/81858#issuecomment-840702765
This is probably a bug, but we don't want to entangle this MR with it.
When it's fixed, this commit should be reverted.
Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
|
|
Closes #73125
This is in pursuance of
Issue #73127 Consider adding #[must_use] to std::process::ExitStatus
In
MR #81452 Add #[must_use] to [...] process::ExitStatus
we concluded that the existing arrangements in are too awkward
so adding that #[must_use] is blocked on improving the ergonomics.
I wrote a mini-RFC-style discusion of the approach in
https://github.com/rust-lang/rust/issues/73125#issuecomment-771092741
Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
|
|
fork fails there. The failure message is confusing: so c.status()
returns an Err, the closure panics, and the test thinks the panic was
propagated from inside the child.
Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
Co-authored-by: Mara Bos <m-ou.se@m-ou.se>
|
|
This tests that we can indeed safely panic after fork, both
a raw libc::fork and in a Command pre_exec hook.
Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
Co-authored-by: Mara Bos <m-ou.se@m-ou.se>
|
|
This is safe (does not involve heap allocation) but we don't yet have
a test to ensure that stays true. That will come in a moment.
Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
Co-authored-by: Mara Bos <m-ou.se@m-ou.se>
|
|
Unwinding after fork() in the child is UB on some platforms, because
on those (including musl) malloc can be UB in the child of a
multithreaded program, and unwinding must box for the payload.
Even if it's safe, unwinding past fork() in the child causes whatever
traps the unwind to return twice. This is very strange and clearly
not desirable. With the default behaviour of the thread library, this
can even result in a panic in the child being transformed into zero
exit status (ie, success) as seen in the parent!
Fixes #79740.
Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
|
|
|
|
|
|
|
|
|
|
This minimizes the size of an unsafe block, and allows outdenting some
complex code.
|
|
ExitStatus: print "exit status: {}" rather than "exit code: {}" on unix
Proper Unix terminology is "exit status" (vs "wait status"). "exit
code" is imprecise on Unix and therefore unclear. (As far as I can
tell, "exit code" is correct terminology on Windows.)
This new wording is unfortunately inconsistent with the identifier
names in the Rust stdlib.
It is the identifier names that are wrong, as discussed at length in eg
https://doc.rust-lang.org/nightly/std/process/struct.ExitStatus.html
https://doc.rust-lang.org/nightly/std/os/unix/process/trait.ExitStatusExt.html
Unfortunately for API stability reasons it would be a lot of work, and
a lot of disruption, to change the names in the stdlib (eg to rename
`std::process::ExitStatus` to `std::process::ChildStatus` or
something), but we should fix the message output. Many (probably
most) readers of these messages about exit statuses will be users and
system administrators, not programmers, who won't even know that Rust
has this wrong terminology.
So I think the right thing is to fix the documentation (as I have
already done) and, now, the terminology in the implementation.
This is a user-visible change to the behaviour of all Rust programs
which run Unix subprocesses. Hopefully no-one is matching against the
exit status string, except perhaps in tests.
|
|
Proper Unix terminology is "exit status" (vs "wait status"). "exit
code" is imprecise on Unix and therefore unclear. (As far as I can
tell, "exit code" is correct terminology on Windows.)
This new wording is unfortunately inconsistent with the identifier
names in the Rust stdlib.
It is the identifier names that are wrong, as discussed at length in eg
https://doc.rust-lang.org/nightly/std/process/struct.ExitStatus.html
https://doc.rust-lang.org/nightly/std/os/unix/process/trait.ExitStatusExt.html
Unfortunately for API stability reasons it would be a lot of work, and
a lot of disruption, to change the names in the stdlib (eg to rename
`std::process::ExitStatus` to `std::process::ChildStatus` or
something), but we should fix the message output. Many (probably
most) readers of these messages about exit statuses will be users and
system administrators, not programmers, who won't even know that Rust
has this wrong terminology.
So I think the right thing is to fix the documentation (as I have
already done) and, now, the terminology in the implementation.
This is a user-visible change to the behaviour of all Rust programs
which run Unix subprocesses. Hopefully no-one is matching against the
exit status string, except perhaps in tests.
Signed-off-by: Ian Jackson <ijackson@chiark.greenend.org.uk>
|
|
Add internal io::Error::new_const to avoid allocations.
This makes it possible to have a io::Error containing a message with zero allocations, and uses that everywhere to avoid the *three* allocations involved in `io::Error::new(kind, "message")`.
The function signature isn't perfect, because it needs a reference to the `&str`. So for now, this is just a `pub(crate)` function. Later, we'll be able to use `fn new_const<MSG: &'static str>(kind: ErrorKind)` to make that a bit better. (Then we'll also be able to use some ZST trickery if that would result in more efficient code.)
See https://github.com/rust-lang/rust/issues/83352
|
|
|
|
This reverts commit acdca316c3d42299d31c1b47eb792006ffdfc29c.
|
|
Do not attempt to unlock envlock in child process after a fork.
This implements the first two points from https://github.com/rust-lang/rust/issues/64718#issuecomment-793030479
This is a breaking change for cases where the environment is accessed in a Command::pre_exec closure. Except for single-threaded programs these uses were not correct anyway since they aren't async-signal safe.
Note that we had a ui test that explicitly tried `env::set_var` in `pre_exec`. As expected it failed with these changes when I tested locally.
|
|
Fixes to ExitStatus and its docs
* On Unix, properly display every possible wait status (and don't panic on weird values)
* In the documentation, be clear and consistent about "exit status" vs "wait status".
|
