Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit c2ec99b

Browse files
committedJun 21, 2024
to extract a pidfd we must consume the child
As long as a pidfd is on a child it can be safely reaped. Taking it would mean the child would now have to be awaited through its pid, but could also be awaited through the pidfd. This could then suffer from a recycling race.
1 parent f7cf777 commit c2ec99b

File tree

3 files changed

+19
-12
lines changed

3 files changed

+19
-12
lines changed
 

‎std/src/os/linux/process.rs

+15-7
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ struct InnerPidFd;
1919
///
2020
/// A `PidFd` can be obtained by setting the corresponding option on [`Command`]
2121
/// with [`create_pidfd`]. Subsequently, the created pidfd can be retrieved
22-
/// from the [`Child`] by calling [`pidfd`] or [`take_pidfd`].
22+
/// from the [`Child`] by calling [`pidfd`] or [`into_pidfd`].
2323
///
2424
/// Example:
2525
/// ```no_run
@@ -33,7 +33,7 @@ struct InnerPidFd;
3333
/// .expect("Failed to spawn child");
3434
///
3535
/// let pidfd = child
36-
/// .take_pidfd()
36+
/// .into_pidfd()
3737
/// .expect("Failed to retrieve pidfd");
3838
///
3939
/// // The file descriptor will be closed when `pidfd` is dropped.
@@ -44,7 +44,7 @@ struct InnerPidFd;
4444
/// [`create_pidfd`]: CommandExt::create_pidfd
4545
/// [`Child`]: process::Child
4646
/// [`pidfd`]: fn@ChildExt::pidfd
47-
/// [`take_pidfd`]: ChildExt::take_pidfd
47+
/// [`into_pidfd`]: ChildExt::into_pidfd
4848
/// [`pidfd_open(2)`]: https://man7.org/linux/man-pages/man2/pidfd_open.2.html
4949
#[derive(Debug)]
5050
#[repr(transparent)]
@@ -159,18 +159,26 @@ pub trait ChildExt: Sealed {
159159
/// [`Child`]: process::Child
160160
fn pidfd(&self) -> Result<&PidFd>;
161161

162-
/// Takes ownership of the [`PidFd`] created for this [`Child`], if available.
162+
/// Returns the [`PidFd`] created for this [`Child`], if available.
163+
/// Otherwise self is returned.
163164
///
164165
/// A pidfd will only be available if its creation was requested with
165166
/// [`create_pidfd`] when the corresponding [`Command`] was created.
166167
///
168+
/// Taking ownership of the PidFd consumes the Child to avoid pid reuse
169+
/// races. Use [`pidfd`] and [`BorrowedFd::try_clone_to_owned`] if
170+
/// you don't want to disassemble the Child yet.
171+
///
167172
/// Even if requested, a pidfd may not be available due to an older
168173
/// version of Linux being in use, or if some other error occurred.
169174
///
170175
/// [`Command`]: process::Command
171176
/// [`create_pidfd`]: CommandExt::create_pidfd
177+
/// [`pidfd`]: ChildExt::pidfd
172178
/// [`Child`]: process::Child
173-
fn take_pidfd(&mut self) -> Result<PidFd>;
179+
fn into_pidfd(self) -> crate::result::Result<PidFd, Self>
180+
where
181+
Self: Sized;
174182
}
175183

176184
/// Os-specific extensions for [`Command`]
@@ -181,7 +189,7 @@ pub trait CommandExt: Sealed {
181189
/// spawned by this [`Command`].
182190
/// By default, no pidfd will be created.
183191
///
184-
/// The pidfd can be retrieved from the child with [`pidfd`] or [`take_pidfd`].
192+
/// The pidfd can be retrieved from the child with [`pidfd`] or [`into_pidfd`].
185193
///
186194
/// A pidfd will only be created if it is possible to do so
187195
/// in a guaranteed race-free manner. Otherwise, [`pidfd`] will return an error.
@@ -195,7 +203,7 @@ pub trait CommandExt: Sealed {
195203
/// [`Command`]: process::Command
196204
/// [`Child`]: process::Child
197205
/// [`pidfd`]: fn@ChildExt::pidfd
198-
/// [`take_pidfd`]: ChildExt::take_pidfd
206+
/// [`into_pidfd`]: ChildExt::into_pidfd
199207
fn create_pidfd(&mut self, val: bool) -> &mut process::Command;
200208
}
201209

‎std/src/sys/pal/unix/linux/pidfd/tests.rs

+2-3
Original file line numberDiff line numberDiff line change
@@ -50,14 +50,13 @@ fn test_pidfd() {
5050
return;
5151
}
5252

53-
let mut child = Command::new("sleep")
53+
let child = Command::new("sleep")
5454
.arg("1000")
5555
.create_pidfd(true)
5656
.spawn()
5757
.expect("executing 'sleep' failed");
5858

59-
let fd = child.take_pidfd().unwrap();
60-
drop(child);
59+
let fd = child.into_pidfd().unwrap();
6160

6261
assert_matches!(fd.try_wait(), Ok(None));
6362
fd.kill().expect("kill failed");

‎std/src/sys/pal/unix/process/process_unix.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -1098,12 +1098,12 @@ mod linux_child_ext {
10981098
.ok_or_else(|| io::Error::new(ErrorKind::Uncategorized, "No pidfd was created."))
10991099
}
11001100

1101-
fn take_pidfd(&mut self) -> io::Result<os::PidFd> {
1101+
fn into_pidfd(mut self) -> Result<os::PidFd, Self> {
11021102
self.handle
11031103
.pidfd
11041104
.take()
11051105
.map(|fd| <os::PidFd as FromInner<imp::PidFd>>::from_inner(fd))
1106-
.ok_or_else(|| io::Error::new(ErrorKind::Uncategorized, "No pidfd was created."))
1106+
.ok_or_else(|| self)
11071107
}
11081108
}
11091109
}

0 commit comments

Comments
 (0)
Failed to load comments.