From fa0b4da5a82839b740eb1563bd2f2e33c7d100b9 Mon Sep 17 00:00:00 2001 From: "Kirill Lukonin (Evil Wireless Man)" Date: Mon, 31 Aug 2026 22:29:31 +0300 Subject: [PATCH 1/2] unix-ffi/os: Reap the child process started by popen(). Previously popen() returned the pipe stream directly and the forked child was never waited for, so it stayed around as a zombie. Return a small wrapper object instead. It forwards stream operations to the pipe, so existing code such as os.popen("/bin/ls").read() keeps working, and it waits for the child when it is closed, either explicitly or at the end of a with statement. As in CPython, close() returns None if the process exited successfully and the exit status otherwise. The child now replaces itself with the command using execvp() instead of calling system(), which removes the extra intermediate shell process. Fixes issue #780. Signed-off-by: Kirill Lukonin (Evil Wireless Man) --- unix-ffi/os/os/__init__.py | 107 +++++++++++++++++++++++++++++++------ 1 file changed, 91 insertions(+), 16 deletions(-) diff --git a/unix-ffi/os/os/__init__.py b/unix-ffi/os/os/__init__.py index 6c87da892..f122e97fe 100644 --- a/unix-ffi/os/os/__init__.py +++ b/unix-ffi/os/os/__init__.py @@ -45,6 +45,7 @@ write_ = libc.func("i", "write", "iPi") close_ = libc.func("i", "close", "i") dup_ = libc.func("i", "dup", "i") + dup2_ = libc.func("i", "dup2", "ii") access_ = libc.func("i", "access", "si") fork_ = libc.func("i", "fork", "") pipe_ = libc.func("i", "pipe", "p") @@ -208,6 +209,12 @@ def dup(fd): return r +def dup2(oldfd, newfd): + r = dup2_(oldfd, newfd) + check_error(r) + return r + + def access(path, mode): return access_(path, mode) == 0 @@ -252,9 +259,12 @@ def getpid(): def waitpid(pid, opts): a = array.array("i", [0]) - r = waitpid_(pid, a, opts) - check_error(r) - return (r, a[0]) + while True: + r = waitpid_(pid, a, opts) + # A signal can interrupt the wait, in which case the child has not been + # reaped yet and the call has to be restarted. + if not check_error(r): + return (r, a[0]) def kill(pid, sig): @@ -294,23 +304,88 @@ def urandom(n): return f.read(n) +class _PopenStream: + # Wraps the pipe to a process started by popen(). Stream operations are + # forwarded to the underlying file, and closing the stream also waits for + # the process to finish, so that it does not stay around as a zombie. + + def __init__(self, f, pid): + self._f = f + self._p = pid + self._s = None + + def __enter__(self): + return self + + def __exit__(self, exc_type, exc_val, exc_tb): + self.close() + + def __iter__(self): + return iter(self._f) + + def read(self, *args): + return self._f.read(*args) + + def readinto(self, *args): + return self._f.readinto(*args) + + def readline(self, *args): + return self._f.readline(*args) + + def readlines(self, *args): + return self._f.readlines(*args) + + def write(self, *args): + return self._f.write(*args) + + def flush(self): + return self._f.flush() + + def fileno(self): + return self._f.fileno() + + def close(self): + if self._f is not None: + f, self._f = self._f, None + try: + f.close() + finally: + # Reap the child even if closing the pipe raised, so that a + # failure there cannot leave the process behind. + _, self._s = waitpid(self._p, 0) + # Match CPython: None if the process exited successfully. + return self._s or None + + def popen(cmd, mode="r"): import builtins - i, o = pipe() - if mode[0] == "w": - i, o = o, i + rfd, wfd = pipe() pid = fork() - if not pid: + + if pid == 0: + # Child: connect the relevant end of the pipe to stdout/stdin, then + # replace this process with the command. if mode[0] == "r": - close(1) + close(rfd) + dup2(wfd, 1) + close(wfd) else: - close(0) - close(i) - dup(o) - close(o) - s = system(cmd) - _exit(s) + close(wfd) + dup2(rfd, 0) + close(rfd) + try: + execvp("sh", ["sh", "-c", cmd]) + except OSError: + pass + _exit(127) + + # Parent: close the child's end of the pipe and wrap the other end so that + # closing it also reaps the child. + if mode[0] == "r": + close(wfd) + fd = rfd else: - close(o) - return builtins.open(i, mode) + close(rfd) + fd = wfd + return _PopenStream(builtins.open(fd, mode), pid) From 2fe840648584fecd92878dc9d550115cd9c8e4aa Mon Sep 17 00:00:00 2001 From: "Kirill Lukonin (Evil Wireless Man)" Date: Mon, 31 Aug 2026 22:29:31 +0300 Subject: [PATCH 2/2] unix-ffi/os: Add a test for popen(). The test covers reading from and writing to a process, use as a context manager, the exit status reported by close(), and that a process which has been closed leaves neither a zombie nor an open file descriptor behind. It is run by CI, which needs the stat package to be installed for the os package to import. Signed-off-by: Kirill Lukonin (Evil Wireless Man) --- tools/ci.sh | 2 ++ unix-ffi/os/test_popen.py | 51 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+) create mode 100644 unix-ffi/os/test_popen.py diff --git a/tools/ci.sh b/tools/ci.sh index 7ee7eb4d1..f31d67c82 100755 --- a/tools/ci.sh +++ b/tools/ci.sh @@ -46,6 +46,7 @@ function ci_package_tests_setup_lib { $CP -r python-stdlib/hashlib-sha384/hashlib "${VIRTUAL_ENV}/lib/" $CP -r python-stdlib/hashlib-sha512/hashlib "${VIRTUAL_ENV}/lib/" $CP python-stdlib/shutil/shutil.py "${VIRTUAL_ENV}/lib/" + $CP python-stdlib/stat/stat.py "${VIRTUAL_ENV}/lib/" $CP python-stdlib/tempfile/tempfile.py "${VIRTUAL_ENV}/lib/" $CP -r python-stdlib/unittest/unittest "${VIRTUAL_ENV}/lib/" $CP -r python-stdlib/unittest-discover/unittest "${VIRTUAL_ENV}/lib/" @@ -77,6 +78,7 @@ function ci_package_tests_run { python-stdlib/string/test_translate.py \ python-stdlib/unittest/tests/exception.py \ unix-ffi/gettext/test_gettext.py \ + unix-ffi/os/test_popen.py \ unix-ffi/pwd/test_getpwnam.py \ unix-ffi/re/test_re.py \ unix-ffi/sqlite3/test_sqlite3.py \ diff --git a/unix-ffi/os/test_popen.py b/unix-ffi/os/test_popen.py new file mode 100644 index 000000000..ba1996cfc --- /dev/null +++ b/unix-ffi/os/test_popen.py @@ -0,0 +1,51 @@ +import os + + +# Read the output of a process. +f = os.popen("echo hello") +assert f.read() == "hello\n" +assert f.close() is None + +# Read the output line by line. +f = os.popen("printf 'a\nb\n'") +assert f.readline() == "a\n" +assert list(f) == ["b\n"] +assert f.close() is None + +# Use the stream as a context manager. +with os.popen("echo hello") as f: + assert f.read() == "hello\n" + +# Write to the input of a process. +with os.popen("cat > test_popen.tmp", "w") as f: + f.write("hello\n") +with open("test_popen.tmp") as f: + assert f.read() == "hello\n" +os.unlink("test_popen.tmp") + +# A non-zero exit status is reported by close(), the same way CPython does it. +f = os.popen("exit 3") +f.read() +assert f.close() == 3 << 8 + +# Closing twice is allowed and reports the same status. +f = os.popen("exit 3") +f.read() +assert f.close() == 3 << 8 +assert f.close() == 3 << 8 + +# The child is reaped by close(), it does not stay around as a zombie, and +# both ends of the pipe are closed, so no file descriptors are leaked either. +children = "/proc/self/task/%d/children" % os.getpid() +if os.access(children, os.F_OK): + fds = len(os.listdir("/proc/self/fd")) + for _ in range(50): + f = os.popen("echo hello") + assert f.read() == "hello\n" + assert f.close() is None + for _ in range(50): + with os.popen("cat > /dev/null", "w") as f: + f.write("hello\n") + with open(children) as f: + assert f.read().split() == [] + assert len(os.listdir("/proc/self/fd")) == fds