diff --git a/Lib/subprocess.py b/Lib/subprocess.py index 1d17ae3608..885f0092b5 100644 --- a/Lib/subprocess.py +++ b/Lib/subprocess.py @@ -74,8 +74,8 @@ else: _mswindows = True -# wasm32-emscripten and wasm32-wasi do not support processes -_can_fork_exec = sys.platform not in {"emscripten", "wasi"} +# some platforms do not support subprocesses +_can_fork_exec = sys.platform not in {"emscripten", "wasi", "ios", "tvos", "watchos"} if _mswindows: import _winapi @@ -83,6 +83,7 @@ STD_INPUT_HANDLE, STD_OUTPUT_HANDLE, STD_ERROR_HANDLE, SW_HIDE, STARTF_USESTDHANDLES, STARTF_USESHOWWINDOW, + STARTF_FORCEONFEEDBACK, STARTF_FORCEOFFFEEDBACK, ABOVE_NORMAL_PRIORITY_CLASS, BELOW_NORMAL_PRIORITY_CLASS, HIGH_PRIORITY_CLASS, IDLE_PRIORITY_CLASS, NORMAL_PRIORITY_CLASS, REALTIME_PRIORITY_CLASS, @@ -93,6 +94,7 @@ "STD_INPUT_HANDLE", "STD_OUTPUT_HANDLE", "STD_ERROR_HANDLE", "SW_HIDE", "STARTF_USESTDHANDLES", "STARTF_USESHOWWINDOW", + "STARTF_FORCEONFEEDBACK", "STARTF_FORCEOFFFEEDBACK", "STARTUPINFO", "ABOVE_NORMAL_PRIORITY_CLASS", "BELOW_NORMAL_PRIORITY_CLASS", "HIGH_PRIORITY_CLASS", "IDLE_PRIORITY_CLASS", @@ -103,18 +105,22 @@ if _can_fork_exec: from _posixsubprocess import fork_exec as _fork_exec # used in methods that are called by __del__ - _waitpid = os.waitpid - _waitstatus_to_exitcode = os.waitstatus_to_exitcode - _WIFSTOPPED = os.WIFSTOPPED - _WSTOPSIG = os.WSTOPSIG - _WNOHANG = os.WNOHANG + class _del_safe: + waitpid = os.waitpid + waitstatus_to_exitcode = os.waitstatus_to_exitcode + WIFSTOPPED = os.WIFSTOPPED + WSTOPSIG = os.WSTOPSIG + WNOHANG = os.WNOHANG + ECHILD = errno.ECHILD else: - _fork_exec = None - _waitpid = None - _waitstatus_to_exitcode = None - _WIFSTOPPED = None - _WSTOPSIG = None - _WNOHANG = None + class _del_safe: + waitpid = None + waitstatus_to_exitcode = None + WIFSTOPPED = None + WSTOPSIG = None + WNOHANG = None + ECHILD = errno.ECHILD + import select import selectors @@ -346,7 +352,7 @@ def _args_from_interpreter_flags(): if dev_mode: args.extend(('-X', 'dev')) for opt in ('faulthandler', 'tracemalloc', 'importtime', - 'frozen_modules', 'showrefcount', 'utf8'): + 'frozen_modules', 'showrefcount', 'utf8', 'gil'): if opt in xoptions: value = xoptions[opt] if value is True: @@ -380,7 +386,7 @@ def _text_encoding(): def call(*popenargs, timeout=None, **kwargs): """Run command with arguments. Wait for command to complete or - timeout, then return the returncode attribute. + for timeout seconds, then return the returncode attribute. The arguments are the same as for the Popen constructor. Example: @@ -517,8 +523,8 @@ def run(*popenargs, in the returncode attribute, and output & stderr attributes if those streams were captured. - If timeout is given, and the process takes too long, a TimeoutExpired - exception will be raised. + If timeout (seconds) is given and the process takes too long, + a TimeoutExpired exception will be raised. There is an optional argument "input", allowing you to pass bytes or a string to the subprocess's stdin. If you use this argument @@ -709,6 +715,9 @@ def _use_posix_spawn(): # os.posix_spawn() is not available return False + if ((_env := os.environ.get('_PYTHON_SUBPROCESS_USE_POSIX_SPAWN')) in ('0', '1')): + return bool(int(_env)) + if sys.platform in ('darwin', 'sunos5'): # posix_spawn() is a syscall on both macOS and Solaris, # and properly reports errors @@ -744,6 +753,7 @@ def _use_posix_spawn(): # guarantee the given libc/syscall API will be used. _USE_POSIX_SPAWN = _use_posix_spawn() _USE_VFORK = True +_HAVE_POSIX_SPAWN_CLOSEFROM = hasattr(os, 'POSIX_SPAWN_CLOSEFROM') class Popen: @@ -834,6 +844,9 @@ def __init__(self, args, bufsize=-1, executable=None, if not isinstance(bufsize, int): raise TypeError("bufsize must be an integer") + if stdout is STDOUT: + raise ValueError("STDOUT can only be used for stderr") + if pipesize is None: pipesize = -1 # Restore default if not isinstance(pipesize, int): @@ -1224,8 +1237,11 @@ def communicate(self, input=None, timeout=None): finally: self._communication_started = True - - sts = self.wait(timeout=self._remaining_time(endtime)) + try: + sts = self.wait(timeout=self._remaining_time(endtime)) + except TimeoutExpired as exc: + exc.timeout = timeout + raise return (stdout, stderr) @@ -1600,6 +1616,10 @@ def _readerthread(self, fh, buffer): fh.close() + def _writerthread(self, input): + self._stdin_write(input) + + def _communicate(self, input, endtime, orig_timeout): # Start reader threads feeding into a list hanging off of this # object, unless they've already been started. @@ -1618,8 +1638,23 @@ def _communicate(self, input, endtime, orig_timeout): self.stderr_thread.daemon = True self.stderr_thread.start() - if self.stdin: - self._stdin_write(input) + # Start writer thread to send input to stdin, unless already + # started. The thread writes input and closes stdin when done, + # or continues in the background on timeout. + if self.stdin and not hasattr(self, "_stdin_thread"): + self._stdin_thread = \ + threading.Thread(target=self._writerthread, + args=(input,)) + self._stdin_thread.daemon = True + self._stdin_thread.start() + + # Wait for the writer thread, or time out. If we time out, the + # thread remains writing and the fd left open in case the user + # calls communicate again. + if hasattr(self, "_stdin_thread"): + self._stdin_thread.join(self._remaining_time(endtime)) + if self._stdin_thread.is_alive(): + raise TimeoutExpired(self.args, orig_timeout) # Wait for the reader threads, or time out. If we time out, the # threads remain reading and the fds left open in case the user @@ -1749,14 +1784,11 @@ def _get_handles(self, stdin, stdout, stderr): errread, errwrite) - def _posix_spawn(self, args, executable, env, restore_signals, + def _posix_spawn(self, args, executable, env, restore_signals, close_fds, p2cread, p2cwrite, c2pread, c2pwrite, errread, errwrite): """Execute program using os.posix_spawn().""" - if env is None: - env = os.environ - kwargs = {} if restore_signals: # See _Py_RestoreSignals() in Python/pylifecycle.c @@ -1778,6 +1810,10 @@ def _posix_spawn(self, args, executable, env, restore_signals, ): if fd != -1: file_actions.append((os.POSIX_SPAWN_DUP2, fd, fd2)) + + if close_fds: + file_actions.append((os.POSIX_SPAWN_CLOSEFROM, 3)) + if file_actions: kwargs['file_actions'] = file_actions @@ -1825,7 +1861,7 @@ def _execute_child(self, args, executable, preexec_fn, close_fds, if (_USE_POSIX_SPAWN and os.path.dirname(executable) and preexec_fn is None - and not close_fds + and (not close_fds or _HAVE_POSIX_SPAWN_CLOSEFROM) and not pass_fds and cwd is None and (p2cread == -1 or p2cread > 2) @@ -1837,7 +1873,7 @@ def _execute_child(self, args, executable, preexec_fn, close_fds, and gids is None and uid is None and umask < 0): - self._posix_spawn(args, executable, env, restore_signals, + self._posix_spawn(args, executable, env, restore_signals, close_fds, p2cread, p2cwrite, c2pread, c2pwrite, errread, errwrite) @@ -1958,20 +1994,16 @@ def _execute_child(self, args, executable, preexec_fn, close_fds, raise child_exception_type(err_msg) - def _handle_exitstatus(self, sts, - _waitstatus_to_exitcode=_waitstatus_to_exitcode, - _WIFSTOPPED=_WIFSTOPPED, - _WSTOPSIG=_WSTOPSIG): + def _handle_exitstatus(self, sts, _del_safe=_del_safe): """All callers to this function MUST hold self._waitpid_lock.""" # This method is called (indirectly) by __del__, so it cannot # refer to anything outside of its local scope. - if _WIFSTOPPED(sts): - self.returncode = -_WSTOPSIG(sts) + if _del_safe.WIFSTOPPED(sts): + self.returncode = -_del_safe.WSTOPSIG(sts) else: - self.returncode = _waitstatus_to_exitcode(sts) + self.returncode = _del_safe.waitstatus_to_exitcode(sts) - def _internal_poll(self, _deadstate=None, _waitpid=_waitpid, - _WNOHANG=_WNOHANG, _ECHILD=errno.ECHILD): + def _internal_poll(self, _deadstate=None, _del_safe=_del_safe): """Check if child process has terminated. Returns returncode attribute. @@ -1987,13 +2019,13 @@ def _internal_poll(self, _deadstate=None, _waitpid=_waitpid, try: if self.returncode is not None: return self.returncode # Another thread waited. - pid, sts = _waitpid(self.pid, _WNOHANG) + pid, sts = _del_safe.waitpid(self.pid, _del_safe.WNOHANG) if pid == self.pid: self._handle_exitstatus(sts) except OSError as e: if _deadstate is not None: self.returncode = _deadstate - elif e.errno == _ECHILD: + elif e.errno == _del_safe.ECHILD: # This happens if SIGCLD is set to be ignored or # waiting for child processes has otherwise been # disabled for our process. This child is dead, we @@ -2067,6 +2099,10 @@ def _communicate(self, input, endtime, orig_timeout): self.stdin.flush() except BrokenPipeError: pass # communicate() must ignore BrokenPipeError. + except ValueError: + # ignore ValueError: I/O operation on closed file. + if not self.stdin.closed: + raise if not input: try: self.stdin.close() @@ -2092,10 +2128,13 @@ def _communicate(self, input, endtime, orig_timeout): self._save_input(input) if self._input: - input_view = memoryview(self._input) + if not isinstance(self._input, memoryview): + input_view = memoryview(self._input) + else: + input_view = self._input.cast("b") # byte input required with _PopenSelector() as selector: - if self.stdin and input: + if self.stdin and not self.stdin.closed and self._input: selector.register(self.stdin, selectors.EVENT_WRITE) if self.stdout and not self.stdout.closed: selector.register(self.stdout, selectors.EVENT_READ) @@ -2128,7 +2167,7 @@ def _communicate(self, input, endtime, orig_timeout): selector.unregister(key.fileobj) key.fileobj.close() else: - if self._input_offset >= len(self._input): + if self._input_offset >= len(input_view): selector.unregister(key.fileobj) key.fileobj.close() elif key.fileobj in (self.stdout, self.stderr): @@ -2137,8 +2176,11 @@ def _communicate(self, input, endtime, orig_timeout): selector.unregister(key.fileobj) key.fileobj.close() self._fileobj2output[key.fileobj].append(data) - - self.wait(timeout=self._remaining_time(endtime)) + try: + self.wait(timeout=self._remaining_time(endtime)) + except TimeoutExpired as exc: + exc.timeout = orig_timeout + raise # All data exchanged. Translate lists into strings. if stdout is not None: diff --git a/Lib/test/test_file.py b/Lib/test/test_file.py index d64f3f797d..36aea52a5a 100644 --- a/Lib/test/test_file.py +++ b/Lib/test/test_file.py @@ -344,9 +344,6 @@ def testIteration(self): class COtherFileTests(OtherFileTests, unittest.TestCase): open = io.open - @unittest.expectedFailure # TODO: RUSTPYTHON - def testSetBufferSize(self): - return super().testSetBufferSize() class PyOtherFileTests(OtherFileTests, unittest.TestCase): open = staticmethod(pyio.open) diff --git a/Lib/test/test_subprocess.py b/Lib/test/test_subprocess.py index 5c5d2a9600..997c98c2c6 100644 --- a/Lib/test/test_subprocess.py +++ b/Lib/test/test_subprocess.py @@ -25,7 +25,6 @@ import gc import textwrap import json -import pathlib from test.support.os_helper import FakePath try: @@ -41,6 +40,10 @@ import grp except ImportError: grp = None +try: + import resource +except ImportError: + resource = None try: import fcntl @@ -158,6 +161,20 @@ def test_call_timeout(self): [sys.executable, "-c", "while True: pass"], timeout=0.1) + def test_timeout_exception(self): + try: + subprocess.run([sys.executable, '-c', 'import time;time.sleep(9)'], timeout = -1) + except subprocess.TimeoutExpired as e: + self.assertIn("-1 seconds", str(e)) + else: + self.fail("Expected TimeoutExpired exception not raised") + try: + subprocess.run([sys.executable, '-c', 'import time;time.sleep(9)'], timeout = 0) + except subprocess.TimeoutExpired as e: + self.assertIn("0 seconds", str(e)) + else: + self.fail("Expected TimeoutExpired exception not raised") + def test_check_call_zero(self): # check_call() function with zero return code rc = subprocess.check_call(ZERO_RETURN_CMD) @@ -269,21 +286,13 @@ def test_check_output_stdin_with_input_arg(self): self.assertIn('stdin', c.exception.args[0]) self.assertIn('input', c.exception.args[0]) - @support.requires_resource('walltime') def test_check_output_timeout(self): # check_output() function with timeout arg with self.assertRaises(subprocess.TimeoutExpired) as c: output = subprocess.check_output( [sys.executable, "-c", - "import sys, time\n" - "sys.stdout.write('BDFL')\n" - "sys.stdout.flush()\n" - "time.sleep(3600)"], - # Some heavily loaded buildbots (sparc Debian 3.x) require - # this much time to start and print. - timeout=3) - self.fail("Expected TimeoutExpired.") - self.assertEqual(c.exception.output, b'BDFL') + "import time; time.sleep(3600)"], + timeout=0.1) def test_call_kwargs(self): # call() function with keyword args @@ -949,6 +958,48 @@ def test_communicate(self): self.assertEqual(stdout, b"banana") self.assertEqual(stderr, b"pineapple") + def test_communicate_memoryview_input(self): + # Test memoryview input with byte elements + test_data = b"Hello, memoryview!" + mv = memoryview(test_data) + p = subprocess.Popen([sys.executable, "-c", + 'import sys; sys.stdout.write(sys.stdin.read())'], + stdin=subprocess.PIPE, + stdout=subprocess.PIPE) + self.addCleanup(p.stdout.close) + self.addCleanup(p.stdin.close) + (stdout, stderr) = p.communicate(mv) + self.assertEqual(stdout, test_data) + self.assertIsNone(stderr) + + def test_communicate_memoryview_input_nonbyte(self): + # Test memoryview input with non-byte elements (e.g., int32) + # This tests the fix for gh-134453 where non-byte memoryviews + # had incorrect length tracking on POSIX + import array + # Create an array of 32-bit integers that's large enough to trigger + # the chunked writing behavior (> PIPE_BUF) + pipe_buf = getattr(select, 'PIPE_BUF', 512) + # Each 'i' element is 4 bytes, so we need more than pipe_buf/4 elements + # Add some extra to ensure we exceed the buffer size + num_elements = pipe_buf + 1 + test_array = array.array('i', [0x64306f66 for _ in range(num_elements)]) + expected_bytes = test_array.tobytes() + mv = memoryview(test_array) + + p = subprocess.Popen([sys.executable, "-c", + 'import sys; ' + 'data = sys.stdin.buffer.read(); ' + 'sys.stdout.buffer.write(data)'], + stdin=subprocess.PIPE, + stdout=subprocess.PIPE) + self.addCleanup(p.stdout.close) + self.addCleanup(p.stdin.close) + (stdout, stderr) = p.communicate(mv) + self.assertEqual(stdout, expected_bytes, + msg=f"{len(stdout)=} =? {len(expected_bytes)=}") + self.assertIsNone(stderr) + def test_communicate_timeout(self): p = subprocess.Popen([sys.executable, "-c", 'import sys,os,time;' @@ -984,6 +1035,62 @@ def test_communicate_timeout_large_output(self): (stdout, _) = p.communicate() self.assertEqual(len(stdout), 4 * 64 * 1024) + def test_communicate_timeout_large_input(self): + # Test that timeout is enforced when writing large input to a + # slow-to-read subprocess, and that partial input is preserved + # for continuation after timeout (gh-141473). + # + # This is a regression test for Windows matching POSIX behavior. + # On POSIX, select() is used to multiplex I/O with timeout checking. + # On Windows, stdin writing must also honor the timeout rather than + # blocking indefinitely when the pipe buffer fills. + + # Input larger than typical pipe buffer (4-64KB on Windows) + input_data = b"x" * (128 * 1024) + + p = subprocess.Popen( + [sys.executable, "-c", + "import sys, time; " + "time.sleep(30); " # Don't read stdin for a long time + "sys.stdout.buffer.write(sys.stdin.buffer.read())"], + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE) + + try: + timeout = 0.2 + start = time.monotonic() + try: + p.communicate(input_data, timeout=timeout) + # If we get here without TimeoutExpired, the timeout was ignored + elapsed = time.monotonic() - start + self.fail( + f"TimeoutExpired not raised. communicate() completed in " + f"{elapsed:.2f}s, but subprocess sleeps for 30s. " + "Stdin writing blocked without enforcing timeout.") + except subprocess.TimeoutExpired: + elapsed = time.monotonic() - start + + # Timeout should occur close to the specified timeout value, + # not after waiting for the subprocess to finish sleeping. + # Allow generous margin for slow CI, but must be well under + # the subprocess sleep time. + self.assertLess(elapsed, 5.0, + f"TimeoutExpired raised after {elapsed:.2f}s; expected ~{timeout}s. " + "Stdin writing blocked without checking timeout.") + + # After timeout, continue communication. The remaining input + # should be sent and we should receive all data back. + stdout, stderr = p.communicate() + + # Verify all input was eventually received by the subprocess + self.assertEqual(len(stdout), len(input_data), + f"Expected {len(input_data)} bytes output but got {len(stdout)}") + self.assertEqual(stdout, input_data) + finally: + p.kill() + p.wait() + # Test for the fd leak reported in http://bugs.python.org/issue2791. def test_communicate_pipe_fd_leak(self): for stdin_pipe in (False, True): @@ -1054,6 +1161,19 @@ def test_writes_before_communicate(self): self.assertEqual(stdout, b"bananasplit") self.assertEqual(stderr, b"") + def test_communicate_stdin_closed_before_call(self): + # gh-70560, gh-74389: stdin.close() before communicate() + # should not raise ValueError from stdin.flush() + with subprocess.Popen([sys.executable, "-c", + 'import sys; sys.exit(0)'], + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE) as p: + p.stdin.close() # Close stdin before communicate + # This should not raise ValueError + (stdout, stderr) = p.communicate() + self.assertEqual(p.returncode, 0) + def test_universal_newlines_and_text(self): args = [ sys.executable, "-c", @@ -1223,6 +1343,16 @@ def test_no_leaking(self): max_handles = 1026 # too much for most UNIX systems else: max_handles = 2050 # too much for (at least some) Windows setups + if resource: + # And if it is not too much, try to make it too much. + try: + soft, hard = resource.getrlimit(resource.RLIMIT_NOFILE) + if soft > 1024: + resource.setrlimit(resource.RLIMIT_NOFILE, (1024, hard)) + self.addCleanup(resource.setrlimit, resource.RLIMIT_NOFILE, + (soft, hard)) + except (OSError, ValueError): + pass handles = [] tmpdir = tempfile.mkdtemp() try: @@ -1237,7 +1367,9 @@ def test_no_leaking(self): else: self.skipTest("failed to reach the file descriptor limit " "(tried %d)" % max_handles) - # Close a couple of them (should be enough for a subprocess) + # Close a couple of them (should be enough for a subprocess). + # Close lower file descriptors, so select() will work. + handles.reverse() for i in range(10): os.close(handles.pop()) # Loop creating some subprocesses. If one of them leaks some fds, @@ -1341,8 +1473,6 @@ def test_bufsize_equal_one_text_mode(self): line = "line\n" self._test_bufsize_equal_one(line, line, universal_newlines=True) - # TODO: RUSTPYTHON - @unittest.expectedFailure def test_bufsize_equal_one_binary_mode(self): # line is not flushed in binary mode with bufsize=1. # we should get empty response @@ -1414,7 +1544,7 @@ def open_fds(): t = threading.Thread(target=open_fds) t.start() try: - with self.assertRaises(EnvironmentError): + with self.assertRaises(OSError): subprocess.Popen(NONEXISTING_CMD, stdin=subprocess.PIPE, stdout=subprocess.PIPE, @@ -1528,9 +1658,6 @@ def test_communicate_epipe(self): p.communicate(b"x" * 2**20) def test_repr(self): - path_cmd = pathlib.Path("my-tool.py") - pathlib_cls = path_cmd.__class__.__name__ - cases = [ ("ls", True, 123, ""), ('a' * 100, True, 0, @@ -1538,7 +1665,8 @@ def test_repr(self): (["ls"], False, None, ""), (["ls", '--my-opts', 'a' * 100], False, None, ""), - (path_cmd, False, 7, f"") + (os_helper.FakePath("my-tool.py"), False, 7, + ">") ] with unittest.mock.patch.object(subprocess.Popen, '_execute_child'): for cmd, shell, code, sx in cases: @@ -1613,21 +1741,6 @@ def test_class_getitems(self): self.assertIsInstance(subprocess.Popen[bytes], types.GenericAlias) self.assertIsInstance(subprocess.CompletedProcess[str], types.GenericAlias) - @unittest.skipIf(not sysconfig.get_config_var("HAVE_VFORK"), - "vfork() not enabled by configure.") - @mock.patch("subprocess._fork_exec") - def test__use_vfork(self, mock_fork_exec): - self.assertTrue(subprocess._USE_VFORK) # The default value regardless. - mock_fork_exec.side_effect = RuntimeError("just testing args") - with self.assertRaises(RuntimeError): - subprocess.run([sys.executable, "-c", "pass"]) - mock_fork_exec.assert_called_once() - self.assertTrue(mock_fork_exec.call_args.args[-1]) - with mock.patch.object(subprocess, '_USE_VFORK', False): - with self.assertRaises(RuntimeError): - subprocess.run([sys.executable, "-c", "pass"]) - self.assertFalse(mock_fork_exec.call_args_list[-1].args[-1]) - @unittest.skipUnless(hasattr(subprocess, '_winapi'), 'need subprocess._winapi') def test_wait_negative_timeout(self): @@ -1644,6 +1757,40 @@ def test_wait_negative_timeout(self): self.assertEqual(proc.wait(), 0) + def test_post_timeout_communicate_sends_input(self): + """GH-141473 regression test; the stdin pipe must close""" + with subprocess.Popen( + [sys.executable, "-uc", """\ +import sys +while c := sys.stdin.read(512): + sys.stdout.write(c) +print() +"""], + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + ) as proc: + try: + data = f"spam{'#'*4096}beans" + proc.communicate( + input=data, + timeout=0, + ) + except subprocess.TimeoutExpired as exc: + pass + # Prior to the bugfix, this would hang as the stdin + # pipe to the child had not been closed. + try: + stdout, stderr = proc.communicate(timeout=15) + except subprocess.TimeoutExpired as exc: + self.fail("communicate() hung waiting on child process that should have seen its stdin pipe close and exit") + self.assertEqual( + proc.returncode, 0, + msg=f"STDERR:\n{stderr}\nSTDOUT:\n{stdout}") + self.assertTrue(stdout.startswith("spam"), msg=stdout) + self.assertIn("beans", stdout) + class RunFuncTestCase(BaseTestCase): def run_python(self, code, **kwargs): @@ -1717,20 +1864,11 @@ def test_check_output_stdin_with_input_arg(self): self.assertIn('stdin', c.exception.args[0]) self.assertIn('input', c.exception.args[0]) - @support.requires_resource('walltime') def test_check_output_timeout(self): with self.assertRaises(subprocess.TimeoutExpired) as c: - cp = self.run_python(( - "import sys, time\n" - "sys.stdout.write('BDFL')\n" - "sys.stdout.flush()\n" - "time.sleep(3600)"), - # Some heavily loaded buildbots (sparc Debian 3.x) require - # this much time to start and print. - timeout=3, stdout=subprocess.PIPE) - self.assertEqual(c.exception.output, b'BDFL') - # output is aliased to stdout - self.assertEqual(c.exception.stdout, b'BDFL') + cp = self.run_python( + "import time; time.sleep(3600)", + timeout=0.1, stdout=subprocess.PIPE) def test_run_kwargs(self): newenv = os.environ.copy() @@ -1785,6 +1923,13 @@ def test_capture_output(self): self.assertIn(b'BDFL', cp.stdout) self.assertIn(b'FLUFL', cp.stderr) + def test_stdout_stdout(self): + # run() refuses to accept stdout=STDOUT + with self.assertRaises(ValueError, + msg=("STDOUT can only be used for stderr")): + self.run_python("print('will not be run')", + stdout=subprocess.STDOUT) + def test_stdout_with_capture_output_arg(self): # run() refuses to accept 'stdout' with 'capture_output' tf = tempfile.TemporaryFile() @@ -1969,8 +2114,6 @@ def bad_error(*args): self.assertIn(repr(error_data), str(e.exception)) - # TODO: RUSTPYTHON - @unittest.expectedFailure @unittest.skipIf(not os.path.exists('/proc/self/status'), "need /proc/self/status") def test_restore_signals(self): @@ -2189,8 +2332,6 @@ def test_extra_groups_invalid_gid_t_values(self): cwd=os.curdir, env=os.environ, extra_groups=[2**64]) - # TODO: RUSTPYTHON - @unittest.expectedFailure @unittest.skipIf(mswindows or not hasattr(os, 'umask'), 'POSIX umask() is not available.') def test_umask(self): @@ -3446,6 +3587,94 @@ def __del__(self): self.assertEqual(out.strip(), b"OK") self.assertIn(b"preexec_fn not supported at interpreter shutdown", err) + @unittest.skipIf(not sysconfig.get_config_var("HAVE_VFORK"), + "vfork() not enabled by configure.") + @mock.patch("subprocess._fork_exec") + @mock.patch("subprocess._USE_POSIX_SPAWN", new=False) + def test__use_vfork(self, mock_fork_exec): + self.assertTrue(subprocess._USE_VFORK) # The default value regardless. + mock_fork_exec.side_effect = RuntimeError("just testing args") + with self.assertRaises(RuntimeError): + subprocess.run([sys.executable, "-c", "pass"]) + mock_fork_exec.assert_called_once() + # NOTE: These assertions are *ugly* as they require the last arg + # to remain the have_vfork boolean. We really need to refactor away + # from the giant "wall of args" internal C extension API. + self.assertTrue(mock_fork_exec.call_args.args[-1]) + with mock.patch.object(subprocess, '_USE_VFORK', False): + with self.assertRaises(RuntimeError): + subprocess.run([sys.executable, "-c", "pass"]) + self.assertFalse(mock_fork_exec.call_args_list[-1].args[-1]) + + @unittest.skipIf(not sysconfig.get_config_var("HAVE_VFORK"), + "vfork() not enabled by configure.") + @unittest.skipIf(sys.platform != "linux", "Linux only, requires strace.") + @mock.patch("subprocess._USE_POSIX_SPAWN", new=False) + def test_vfork_used_when_expected(self): + # This is a performance regression test to ensure we default to using + # vfork() when possible. + # Technically this test could pass when posix_spawn is used as well + # because libc tends to implement that internally using vfork. But + # that'd just be testing a libc+kernel implementation detail. + strace_binary = "/usr/bin/strace" + # The only system calls we are interested in. + strace_filter = "--trace=clone,clone2,clone3,fork,vfork,exit,exit_group" + true_binary = "/bin/true" + strace_command = [strace_binary, strace_filter] + + try: + does_strace_work_process = subprocess.run( + strace_command + [true_binary], + stderr=subprocess.PIPE, + stdout=subprocess.DEVNULL, + ) + rc = does_strace_work_process.returncode + stderr = does_strace_work_process.stderr + except OSError: + rc = -1 + stderr = "" + if rc or (b"+++ exited with 0 +++" not in stderr): + self.skipTest("strace not found or not working as expected.") + + with self.subTest(name="default_is_vfork"): + vfork_result = assert_python_ok( + "-c", + textwrap.dedent(f"""\ + import subprocess + subprocess.check_call([{true_binary!r}])"""), + __run_using_command=strace_command, + ) + # Match both vfork() and clone(..., flags=...|CLONE_VFORK|...) + self.assertRegex(vfork_result.err, br"(?i)vfork") + # Do NOT check that fork() or other clones did not happen. + # If the OS denys the vfork it'll fallback to plain fork(). + + # Test that each individual thing that would disable the use of vfork + # actually disables it. + for sub_name, preamble, sp_kwarg, expect_permission_error in ( + ("!use_vfork", "subprocess._USE_VFORK = False", "", False), + ("preexec", "", "preexec_fn=lambda: None", False), + ("setgid", "", f"group={os.getgid()}", True), + ("setuid", "", f"user={os.getuid()}", True), + ("setgroups", "", "extra_groups=[]", True), + ): + with self.subTest(name=sub_name): + non_vfork_result = assert_python_ok( + "-c", + textwrap.dedent(f"""\ + import subprocess + {preamble} + try: + subprocess.check_call( + [{true_binary!r}], **dict({sp_kwarg})) + except PermissionError: + if not {expect_permission_error}: + raise"""), + __run_using_command=strace_command, + ) + # Ensure neither vfork() or clone(..., flags=...|CLONE_VFORK|...). + self.assertNotRegex(non_vfork_result.err, br"(?i)vfork") + @unittest.skipUnless(mswindows, "Windows specific tests") class Win32ProcessTestCase(BaseTestCase): diff --git a/crates/stdlib/src/posixsubprocess.rs b/crates/stdlib/src/posixsubprocess.rs index d05b24fd6d..4b895d2102 100644 --- a/crates/stdlib/src/posixsubprocess.rs +++ b/crates/stdlib/src/posixsubprocess.rs @@ -321,11 +321,14 @@ fn exec_inner( } if args.child_umask >= 0 { - // TODO: umask(child_umask); + unsafe { libc::umask(args.child_umask as libc::mode_t) }; } if args.restore_signals { - // TODO: restore signals SIGPIPE, SIGXFZ, SIGXFSZ to SIG_DFL + unsafe { + libc::signal(libc::SIGPIPE, libc::SIG_DFL); + libc::signal(libc::SIGXFSZ, libc::SIG_DFL); + } } if args.call_setsid { diff --git a/crates/vm/src/stdlib/io.rs b/crates/vm/src/stdlib/io.rs index 89dc8bca92..52900faec0 100644 --- a/crates/vm/src/stdlib/io.rs +++ b/crates/vm/src/stdlib/io.rs @@ -4489,6 +4489,16 @@ mod _io { bool::try_from_object(vm, atty)? }; + // Warn if line buffering is requested in binary mode + if opts.buffering == 1 && matches!(mode.encode, EncodeMode::Bytes) { + crate::stdlib::warnings::warn( + vm.ctx.exceptions.runtime_warning, + "line buffering (buffering=1) isn't supported in binary mode, the default buffer size will be used".to_owned(), + 1, + vm, + )?; + } + let line_buffering = opts.buffering == 1 || isatty; let buffering = if opts.buffering < 0 || opts.buffering == 1 { diff --git a/crates/vm/src/stdlib/posix.rs b/crates/vm/src/stdlib/posix.rs index efbd0cf904..a4a311df06 100644 --- a/crates/vm/src/stdlib/posix.rs +++ b/crates/vm/src/stdlib/posix.rs @@ -1515,7 +1515,7 @@ pub mod module { #[pyarg(positional)] args: crate::function::ArgIterable, #[pyarg(positional)] - env: crate::function::ArgMapping, + env: Option, #[pyarg(named, default)] file_actions: Option>, #[pyarg(named, default)] @@ -1678,7 +1678,22 @@ pub mod module { .map_err(|_| vm.new_value_error("path should not have nul bytes")) }) .collect::>()?; - let env = envp_from_dict(self.env, vm)?; + let env = if let Some(env_dict) = self.env { + envp_from_dict(env_dict, vm)? + } else { + // env=None means use the current environment + use rustpython_common::os::ffi::OsStringExt; + env::vars_os() + .map(|(k, v)| { + let mut entry = k.into_vec(); + entry.push(b'='); + entry.extend(v.into_vec()); + CString::new(entry).map_err(|_| { + vm.new_value_error("environment string contains null byte") + }) + }) + .collect::>>()? + }; let ret = if spawnp { nix::spawn::posix_spawnp(&path, &file_actions, &attrp, &args, &env) diff --git a/crates/vm/src/stdlib/winapi.rs b/crates/vm/src/stdlib/winapi.rs index 5cfb62fad6..a3aba44748 100644 --- a/crates/vm/src/stdlib/winapi.rs +++ b/crates/vm/src/stdlib/winapi.rs @@ -63,7 +63,8 @@ mod _winapi { CREATE_BREAKAWAY_FROM_JOB, CREATE_DEFAULT_ERROR_MODE, CREATE_NEW_CONSOLE, CREATE_NEW_PROCESS_GROUP, CREATE_NO_WINDOW, DETACHED_PROCESS, HIGH_PRIORITY_CLASS, IDLE_PRIORITY_CLASS, INFINITE, NORMAL_PRIORITY_CLASS, PROCESS_DUP_HANDLE, - REALTIME_PRIORITY_CLASS, STARTF_USESHOWWINDOW, STARTF_USESTDHANDLES, + REALTIME_PRIORITY_CLASS, STARTF_FORCEOFFFEEDBACK, STARTF_FORCEONFEEDBACK, + STARTF_USESHOWWINDOW, STARTF_USESTDHANDLES, }, }, UI::WindowsAndMessaging::SW_HIDE,