From c7fcd565ed61d1e8b3610fd72538700f11d5f8f9 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Wed, 24 Dec 2025 15:32:23 +0900 Subject: [PATCH] impl preexec_fn --- Lib/test/test_subprocess.py | 12 ----------- crates/stdlib/src/posixsubprocess.rs | 32 ++++++++++++++++++++++------ 2 files changed, 25 insertions(+), 19 deletions(-) diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py index e04f8b8fcc4..3917c0a76d9 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -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. @@ -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?") @@ -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.""" @@ -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): @@ -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") @@ -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") diff --git a/crates/stdlib/src/posixsubprocess.rs b/crates/stdlib/src/posixsubprocess.rs index 4da6a6858dd..d05b24fd6dd 100644 --- a/crates/stdlib/src/posixsubprocess.rs +++ b/crates/stdlib/src/posixsubprocess.rs @@ -33,9 +33,6 @@ mod _posixsubprocess { #[pyfunction] fn fork_exec(args: ForkExecArgs<'_>, vm: &VirtualMachine) -> PyResult { - 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() @@ -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()), } } @@ -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) } } @@ -242,6 +245,7 @@ fn exec(args: &ForkExecArgs<'_>, procargs: ProcArgs<'_>) -> ! { enum ExecErrorContext { NoExec, ChDir, + PreExec, Exec, } @@ -250,6 +254,7 @@ impl ExecErrorContext { match self { Self::NoExec => "noexec", Self::ChDir => "noexec:chdir", + Self::PreExec => "Exception occurred in preexec_fn.", Self::Exec => "", } } @@ -259,6 +264,7 @@ fn exec_inner( args: &ForkExecArgs<'_>, procargs: ProcArgs<'_>, ctx: &mut ExecErrorContext, + vm: &VirtualMachine, ) -> nix::Result { for &fd in args.fds_to_keep.as_slice() { if fd.as_raw_fd() != args.errpipe_write.as_raw_fd() { @@ -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 {