Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rollup of 7 pull requests #83119

Closed
wants to merge 20 commits into from

Conversation

Dylan-DPC-zz
Copy link

Successful merges:

Failed merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

Aaron1011 and others added 20 commits March 10, 2021 23:13
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?
Co-authored-by: bjorn3 <bjorn3@users.noreply.github.com>
Merges `if-let` and `if x.is_some() { ... }` blocks
Add Linux-specific pidfd process extensions (take 2)

Continuation of rust-lang#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: rust-lang#82971
expand: Resolve and expand inner attributes on out-of-line modules

Fixes rust-lang#81661
r? `@Aaron1011`
…ange, r=davidtwco

Validate rustc_layout_scalar_valid_range_{start,end} attributes

Fixes rust-lang#82251, rust-lang#82981.
…avidtwco

Improve the wording for the `can't reassign` error

Follow-up for rust-lang#71976 (comment).
Fixes rust-lang#66736
More precise spans for HIR paths

`Ty::assoc_item` is lowered to `<Ty>::assoc_item` in HIR, but `Ty` got span from the whole path.
This PR fixes that, and adjusts some diagnostic code that relied on `Ty` having the whole path span.

This is a pre-requisite for rust-lang#82868 (we cannot report suggestions like `Tr::assoc` -> `<dyn Tr>::assoc` with the current imprecise spans).
r? `@estebank`
Fix typos in `library/core/src/ptr/mod.rs` and `library/std/src/sys_common/thread_local_dtor.rs`

adress -> address
…as-schievink

Minor refactoring in try_index_step

Merges `if-let` and `if x.is_some() { ... }` blocks
@rustbot rustbot added the rollup A PR which is a rollup label Mar 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rollup A PR which is a rollup
Projects
None yet
Development

Successfully merging this pull request may close these issues.