impl preexec_fn (#6479)

youknowone · web-flow · commit 4f0b940b1607 · 2025-12-24T17:02:21.000+09:00
diff --git 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
@@ -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()
@@ -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<Never> {
     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 {
