Skip to content

process: add dup2 file action#153133

Open
avikivity wants to merge 1 commit intorust-lang:mainfrom
avikivity:unix-spawn-dup2
Open

process: add dup2 file action#153133
avikivity wants to merge 1 commit intorust-lang:mainfrom
avikivity:unix-spawn-dup2

Conversation

@avikivity
Copy link

Support the posix_spawn_file_actions_adddup2() library call.

The direct motivation for this is the jobserver crate using pre_exec to clear the CLOEXEC flag on the jobserver fd. This disables the vfork optimization and slows down posix_spawn() greatly.

Support the posix_spawn_file_actions_adddup2() library call.

The direct motivation for this is the jobserver crate using pre_exec
to clear the CLOEXEC flag on the jobserver fd. This disables the vfork
optimization and slows down posix_spawn() greatly.
@rustbot rustbot added O-unix Operating system: Unix-like S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Feb 26, 2026
@rustbot
Copy link
Collaborator

rustbot commented Feb 26, 2026

r? @petrochenkov

rustbot has assigned @petrochenkov.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: compiler
  • compiler expanded to 68 candidates
  • Random selection from 14 candidates

@avikivity
Copy link
Author

Disclosure: I was assisted by AI to prepare the patch. I believe it is of good quality.

@petrochenkov
Copy link
Contributor

r? library

@rustbot
Copy link
Collaborator

rustbot commented Feb 26, 2026

Failed to set assignee to library: invalid assignee

Note: Only org members with at least the repository "read" role, users with write permissions, or people who have commented on the PR may be assigned.

@petrochenkov
Copy link
Contributor

r? libs

@rustbot rustbot assigned joboet and unassigned petrochenkov Feb 26, 2026
@Kivooeo Kivooeo removed the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Feb 26, 2026
@joboet
Copy link
Member

joboet commented Feb 26, 2026

Disclosure: I was assisted by AI to prepare the patch. I believe it is of good quality.

Thank you for the honesty!

Was this discussed previously by T-libs-api? In any case, I have some questions about the API:

  • I'm not a big fan of the name dup2_file_action – how about something like set_fd?
  • Changing the file descriptor table of the spawned process isn't relevant for I/O safety in the current process. So if you accept BorrowedFd or (even better) impl AsFd as a first argument, the function could be made safe (although it's probably advisable to check for conflicts between actions).
  • It may be nice to extract the "clear-CLOEXEC" case to a separate function like inherit_fd or something like that (that definitely could be made safe).

And also,
r? libs-api

@rustbot rustbot added the T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. label Feb 26, 2026
@rustbot rustbot assigned Amanieu and unassigned joboet Feb 26, 2026
@bjorn3
Copy link
Member

bjorn3 commented Feb 27, 2026

How is this different from #145687?

@avikivity
Copy link
Author

How is this different from #145687?

At least:

  • this one is documented, the other has to be reverse-engineered to understand what it does
  • this one is being maintained, the other appears abandoned

It's possible that the contents are equivalent.

@avikivity
Copy link
Author

Disclosure: I was assisted by AI to prepare the patch. I believe it is of good quality.

Thank you for the honesty!

Was this discussed previously by T-libs-api?

I don't know. I don't even know what a T-libs-api is. I'm not a regular Rust contributor.

In any case, I have some questions about the API:

  • I'm not a big fan of the name dup2_file_action – how about something like set_fd?

I think dup2_file_action, while clumsy, is better. We're not implementing some general computing concept, we're plumbing an existing API. It's likely users will approach it from the point of "how do I push a dup2_file_action through the process API (coming from below, the Linux direction) rather than "let's explore the process API and see what tools it gives me" (coming from above, the application direction). It solves a very low-level problem.

That said, I'll happily adjust to conform to the library's wishes.

  • Changing the file descriptor table of the spawned process isn't relevant for I/O safety in the current process. So if you accept BorrowedFd or (even better) impl AsFd as a first argument, the function could be made safe (although it's probably advisable to check for conflicts between actions).

Understood, I'll try it.

  • It may be nice to extract the "clear-CLOEXEC" case to a separate function like inherit_fd or something like that (that definitely could be made safe).

Ok.

And also, r? libs-api

Awaiting your preference on the naming.

@bjorn3
Copy link
Member

bjorn3 commented Mar 1, 2026

I think dup2_file_action, while clumsy, is better. We're not implementing some general computing concept, we're plumbing an existing API.

It wouldn't use posix_spawn_file_actions_adddup2 if fork+exec (or just exec if the user wanted to replace the current process). posix_spawn_file_actions_adddup2 is an implementation detail of the actual operation that this does: Passing an fd to the new process.

///
/// [`pre_exec`]: CommandExt::pre_exec
#[unstable(feature = "process_file_actions", issue = "none")]
unsafe fn dup2_file_action(&mut self, oldfd: RawFd, newfd: RawFd) -> &mut process::Command;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this accept an OwnedFd or BorrowedFd as input fd to make this function safe?

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

Labels

O-unix Operating system: Unix-like S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants