Skip to content

impl preexec_fn #6479

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.

Sign up for GitHub

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

Merged
youknowone merged 1 commit into from
Dec 24, 2025
Merged

impl preexec_fn #6479

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 0 additions & 12 deletions Lib/test/test_subprocess.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2244,8 +2244,6 @@ def test_CalledProcessError_str_non_zero(self):
error_string = str(err)
self.assertIn("non-zero exit status 2.", error_string)

# TODO: RUSTPYTHON
@unittest.expectedFailure
def test_preexec(self):
# DISCLAIMER: Setting environment variables is *not* a good use
# of a preexec_fn. This is merely a test.
Expand All @@ -2257,8 +2255,6 @@ def test_preexec(self):
with p:
self.assertEqual(p.stdout.read(), b"apple")

# TODO: RUSTPYTHON
@unittest.expectedFailure
def test_preexec_exception(self):
def raise_it():
raise ValueError("What if two swallows carried a coconut?")
Expand Down Expand Up @@ -2300,8 +2296,6 @@ def _execute_child(self, *args, **kwargs):
for fd in devzero_fds:
os.close(fd)

# TODO: RUSTPYTHON
@unittest.expectedFailure
@unittest.skipIf(not os.path.exists("/dev/zero"), "/dev/zero required.")
def test_preexec_errpipe_does_not_double_close_pipes(self):
"""Issue16140: Don't double close pipes on preexec error."""
Expand Down Expand Up @@ -2339,8 +2333,6 @@ def test_preexec_gc_module_failure(self):
if not enabled:
gc.disable()

# TODO: RUSTPYTHON
@unittest.expectedFailure
@unittest.skipIf(
sys.platform == 'darwin', 'setrlimit() seems to fail on OS X')
def test_preexec_fork_failure(self):
Expand Down Expand Up @@ -2751,8 +2743,6 @@ def test_swap_std_fds_with_one_closed(self):
for to_fds in itertools.permutations(range(3), 2):
self._check_swap_std_fds_with_one_closed(from_fds, to_fds)

# TODO: RUSTPYTHON
@unittest.expectedFailure
def test_surrogates_error_message(self):
def prepare():
raise ValueError("surrogate:\uDCff")
Expand Down Expand Up @@ -3228,8 +3218,6 @@ def test_leak_fast_process_del_killed(self):
else:
self.assertNotIn(ident, [id(o) for o in subprocess._active])

# TODO: RUSTPYTHON
@unittest.expectedFailure
def test_close_fds_after_preexec(self):
fd_status = support.findfile("fd_status.py", subdir="subprocessdata")

Expand Down
32 changes: 25 additions & 7 deletions crates/stdlib/src/posixsubprocess.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,6 @@ mod _posixsubprocess {

#[pyfunction]
fn fork_exec(args: ForkExecArgs<'_>, vm: &VirtualMachine) -> PyResult<libc::pid_t> {
if args.preexec_fn.is_some() {
return Err(vm.new_not_implemented_error("preexec_fn not supported yet"));
}
let extra_groups = args
.groups_list
.as_ref()
Expand All @@ -49,7 +46,7 @@ mod _posixsubprocess {
extra_groups: extra_groups.as_deref(),
};
match unsafe { nix::unistd::fork() }.map_err(|err| err.into_pyexception(vm))? {
nix::unistd::ForkResult::Child => exec(&args, procargs),
nix::unistd::ForkResult::Child => exec(&args, procargs, vm),
nix::unistd::ForkResult::Parent { child } => Ok(child.as_raw()),
}
}
Expand Down Expand Up @@ -227,13 +224,19 @@ struct ProcArgs<'a> {
extra_groups: Option<&'a [Gid]>,
}

fn exec(args: &ForkExecArgs<'_>, procargs: ProcArgs<'_>) -> ! {
fn exec(args: &ForkExecArgs<'_>, procargs: ProcArgs<'_>, vm: &VirtualMachine) -> ! {
let mut ctx = ExecErrorContext::NoExec;
match exec_inner(args, procargs, &mut ctx) {
match exec_inner(args, procargs, &mut ctx, vm) {
Ok(x) => match x {},
Err(e) => {
let mut pipe = args.errpipe_write;
let _ = write!(pipe, "OSError:{}:{}", e as i32, ctx.as_msg());
if matches!(ctx, ExecErrorContext::PreExec) {
// For preexec_fn errors, use SubprocessError format (errno=0)
let _ = write!(pipe, "SubprocessError:0:{}", ctx.as_msg());
} else {
// errno is written in hex format
let _ = write!(pipe, "OSError:{:x}:{}", e as i32, ctx.as_msg());
}
std::process::exit(255)
}
}
Expand All @@ -242,6 +245,7 @@ fn exec(args: &ForkExecArgs<'_>, procargs: ProcArgs<'_>) -> ! {
enum ExecErrorContext {
NoExec,
ChDir,
PreExec,
Exec,
}

Expand All @@ -250,6 +254,7 @@ impl ExecErrorContext {
match self {
Self::NoExec => "noexec",
Self::ChDir => "noexec:chdir",
Self::PreExec => "Exception occurred in preexec_fn.",
Self::Exec => "",
}
}
Expand All @@ -259,6 +264,7 @@ fn exec_inner(
args: &ForkExecArgs<'_>,
procargs: ProcArgs<'_>,
ctx: &mut ExecErrorContext,
vm: &VirtualMachine,
) -> nix::Result<Never> {
for &fd in args.fds_to_keep.as_slice() {
if fd.as_raw_fd() != args.errpipe_write.as_raw_fd() {
Expand Down Expand Up @@ -345,6 +351,18 @@ fn exec_inner(
nix::Error::result(ret)?;
}

// Call preexec_fn after all process setup but before closing FDs
if let Some(ref preexec_fn) = args.preexec_fn {
match preexec_fn.call((), vm) {
Ok(_) => {}
Err(_e) => {
// Cannot safely stringify exception after fork
*ctx = ExecErrorContext::PreExec;
return Err(Errno::UnknownErrno);
}
}
}

*ctx = ExecErrorContext::Exec;

if args.close_fds {
Expand Down
Loading