Skip to content

Commit

Permalink
Merge pull request #23 from google/fd_mappings
Browse files Browse the repository at this point in the history
Update documentation of `fd_mappings`
  • Loading branch information
qwandor committed Dec 1, 2023
2 parents e0b2986 + dc67955 commit fca5c8a
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 8 deletions.
13 changes: 7 additions & 6 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,10 @@ pub struct FdMappingCollision;
pub trait CommandFdExt {
/// Adds the given set of file descriptors to the command.
///
/// Warning: Calling this more than once on the same command, or attempting to run the same
/// command more than once after calling this, may result in unexpected behaviour.
/// Warning: Calling this more than once on the same command may result in unexpected behaviour.
/// In particular, it is not possible to check that two mappings applied separately don't use
/// the same `child_fd`. If there is such a collision then one will apply and the other will be
/// lost.
fn fd_mappings(&mut self, mappings: Vec<FdMapping>) -> Result<&mut Self, FdMappingCollision>;

/// Adds the given set of file descriptors to be passed on to the child process when the command
Expand All @@ -98,10 +100,9 @@ impl CommandFdExt for Command {
// Register the callback to apply the mappings after forking but before execing.
// Safety: `map_fds` will not allocate, so it is safe to call from this hook.
unsafe {
// If the command is run more than once, and hence this closure is called multiple
// times, then `mappings` may be in an incorrect state. It would be good if we could
// reset it to the initial state somehow, or use something else for saving the temporary
// mappings.
// If the command is run more than once, the closure will be called multiple times but
// in different forked processes, which will have different copies of `mappings`. So
// their changes to it shouldn't be visible to each other.
self.pre_exec(move || map_fds(&mut mappings, &child_fds));
}

Expand Down
6 changes: 4 additions & 2 deletions src/tokio.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,10 @@ use crate::{map_fds, preserve_fds, validate_child_fds, FdMapping, FdMappingColli
pub trait CommandFdAsyncExt {
/// Adds the given set of file descriptors to the command.
///
/// Warning: Calling this more than once on the same command, or attempting to run the same
/// command more than once after calling this, may result in unexpected behaviour.
/// Warning: Calling this more than once on the same command may result in unexpected behaviour.
/// In particular, it is not possible to check that two mappings applied separately don't use
/// the same `child_fd`. If there is such a collision then one will apply and the other will be
/// lost.
fn fd_mappings(&mut self, mappings: Vec<FdMapping>) -> Result<&mut Self, FdMappingCollision>;

/// Adds the given set of file descriptors to be passed on to the child process when the command
Expand Down

0 comments on commit fca5c8a

Please sign in to comment.