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

Add new linux-only API process::Child::pidfd #6281

Open
NobodyXu opened this issue Jan 11, 2024 · 3 comments
Open

Add new linux-only API process::Child::pidfd #6281

NobodyXu opened this issue Jan 11, 2024 · 3 comments
Labels
A-tokio Area: The main tokio crate C-feature-request Category: A feature request. M-process Module: tokio/process

Comments

@NobodyXu
Copy link
Contributor

NobodyXu commented Jan 11, 2024

Is your feature request related to a problem? Please describe.

Now that #6152 is merged, I think it's reasonable to expose the underlying pidfd we use to the user since AFAIK there are two syscalls that must take pidfd to work:

  • process_madvise: madvice but for another process if the caller has PTRACE_MODE_READ_FSCREDS and CAP_SYS_NICE
  • process_vm_readv: read or write virtual memory of another process if the caller has PTRACE_MODE_ATTACH_REALCREDS

Describe the solution you'd like

use std::os::fd::BorrowedFd;

impl Child {
    #[cfg(linux)]
    fn pidfd(&self) -> Option<BorrowedFd<'_>>;
}

This API is modeled after unstable libstd API std::os::linux::process::ChildExt:

pub trait ChildExt: Sealed {
    // Required methods
    fn pidfd(&self) -> Result<&PidFd>;
    fn take_pidfd(&mut self) -> Result<PidFd>;
}

Describe alternatives you've considered

Additional context

@NobodyXu NobodyXu added A-tokio Area: The main tokio crate C-feature-request Category: A feature request. labels Jan 11, 2024
@Darksonn Darksonn added the M-process Module: tokio/process label Jan 11, 2024
@Darksonn
Copy link
Contributor

Hmm. This means that we would be using a different return type than std for the pidfd method. Perhaps it would make sense to wait until std stabilizes this?

That said, adding the API under --cfg tokio_unstable would not be a problem.

@NobodyXu
Copy link
Contributor Author

Hmm. This means that we would be using a different return type than std for the pidfd method. Perhaps it would make sense to wait until std stabilizes this?

That's right, but the PidFd in libstd is just an owned file with AsFd, it doesn't have any method.

The discussion in the tracking issue seems to be focused on how to obtain a pidfd in a race-free way for now, maybe I should get involved and raise that question again in the issue.

@Darksonn
Copy link
Contributor

Okay. Let's wait a bit with adding this API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate C-feature-request Category: A feature request. M-process Module: tokio/process
Projects
None yet
Development

No branches or pull requests

2 participants