From 1d8930957a8c0777f9d6b13fcb1b296df029c132 Mon Sep 17 00:00:00 2001 From: Manjusaka Date: Mon, 29 Sep 2025 14:14:40 +0800 Subject: [PATCH 1/7] gh-139184: set O_CLOEXEC for master_fd when call os.forkpty Signed-off-by: Manjusaka --- Lib/test/test_pty.py | 4 ++++ .../2025-09-29-14-15-20.gh-issue-139184.dNl9O4.rst | 1 + Modules/posixmodule.c | 8 ++++++++ 3 files changed, 13 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2025-09-29-14-15-20.gh-issue-139184.dNl9O4.rst diff --git a/Lib/test/test_pty.py b/Lib/test/test_pty.py index fbba7025ac4abf..65185613fae337 100644 --- a/Lib/test/test_pty.py +++ b/Lib/test/test_pty.py @@ -12,6 +12,7 @@ raise unittest.SkipTest("pty is not available on this platform") import errno +import fcntl import os import pty import tty @@ -230,6 +231,9 @@ def test_fork(self): os._exit(2) os._exit(4) else: + flags = fcntl.fcntl(master_fd, fcntl.F_GETFD) + cloexec_set = bool(flags & fcntl.FD_CLOEXEC) + self.assertEqual(cloexec_set, True) debug("Waiting for child (%d) to finish." % pid) # In verbose mode, we have to consume the debug output from the # child or the child will block, causing this test to hang in the diff --git a/Misc/NEWS.d/next/Library/2025-09-29-14-15-20.gh-issue-139184.dNl9O4.rst b/Misc/NEWS.d/next/Library/2025-09-29-14-15-20.gh-issue-139184.dNl9O4.rst new file mode 100644 index 00000000000000..7388afaa94ad48 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2025-09-29-14-15-20.gh-issue-139184.dNl9O4.rst @@ -0,0 +1 @@ +Set O_CLOEXEC for master_fd when call os.forkpty diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index b7a0110226590e..be2b501e4fa889 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -9059,6 +9059,9 @@ os_forkpty_impl(PyObject *module) } else { /* parent: release the import lock. */ PyOS_AfterFork_Parent(); + /* set O_CLOEXEC on master_fd */ + if (_Py_set_inheritable(master_fd, 0, NULL) < 0) + goto error; // After PyOS_AfterFork_Parent() starts the world to avoid deadlock. if (warn_about_fork_with_threads("forkpty") < 0) return NULL; @@ -9066,7 +9069,12 @@ os_forkpty_impl(PyObject *module) if (pid == -1) { return posix_error(); } + return Py_BuildValue("(Ni)", PyLong_FromPid(pid), master_fd); +error: + if (master_fd != -1) + close(master_fd); + return NULL; } #endif /* HAVE_FORKPTY */ From b0cf98a317f733bbb3231327858bcd22ab005247 Mon Sep 17 00:00:00 2001 From: Nadeshiko Manju Date: Mon, 29 Sep 2025 17:42:34 +0800 Subject: [PATCH 2/7] Update Lib/test/test_pty.py Co-authored-by: Shamil --- Lib/test/test_pty.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_pty.py b/Lib/test/test_pty.py index 65185613fae337..98e8c28d605d45 100644 --- a/Lib/test/test_pty.py +++ b/Lib/test/test_pty.py @@ -233,7 +233,8 @@ def test_fork(self): else: flags = fcntl.fcntl(master_fd, fcntl.F_GETFD) cloexec_set = bool(flags & fcntl.FD_CLOEXEC) - self.assertEqual(cloexec_set, True) + flags = fcntl.fcntl(master_fd, fcntl.F_GETFD) + self.assertTrue(flags & fcntl.FD_CLOEXEC) debug("Waiting for child (%d) to finish." % pid) # In verbose mode, we have to consume the debug output from the # child or the child will block, causing this test to hang in the From d9be40e62f10776951a4e52a692d30c20122143e Mon Sep 17 00:00:00 2001 From: Manjusaka Date: Mon, 29 Sep 2025 17:44:05 +0800 Subject: [PATCH 3/7] fix test Signed-off-by: Manjusaka --- Lib/test/test_pty.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/Lib/test/test_pty.py b/Lib/test/test_pty.py index 98e8c28d605d45..385169efcd5b1d 100644 --- a/Lib/test/test_pty.py +++ b/Lib/test/test_pty.py @@ -231,8 +231,6 @@ def test_fork(self): os._exit(2) os._exit(4) else: - flags = fcntl.fcntl(master_fd, fcntl.F_GETFD) - cloexec_set = bool(flags & fcntl.FD_CLOEXEC) flags = fcntl.fcntl(master_fd, fcntl.F_GETFD) self.assertTrue(flags & fcntl.FD_CLOEXEC) debug("Waiting for child (%d) to finish." % pid) From 74b77d3c44c23457064f99a7c7fe66966982747e Mon Sep 17 00:00:00 2001 From: Manjusaka Date: Thu, 9 Oct 2025 01:40:03 +0800 Subject: [PATCH 4/7] fix review idea Signed-off-by: Manjusaka --- Lib/test/test_pty.py | 3 +-- .../2025-09-29-14-15-20.gh-issue-139184.dNl9O4.rst | 2 +- Modules/posixmodule.c | 11 +++++------ 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/Lib/test/test_pty.py b/Lib/test/test_pty.py index 385169efcd5b1d..966e19b9d65a50 100644 --- a/Lib/test/test_pty.py +++ b/Lib/test/test_pty.py @@ -231,8 +231,7 @@ def test_fork(self): os._exit(2) os._exit(4) else: - flags = fcntl.fcntl(master_fd, fcntl.F_GETFD) - self.assertTrue(flags & fcntl.FD_CLOEXEC) + self.assertFalse(os.get_inheritable(master_fd)) debug("Waiting for child (%d) to finish." % pid) # In verbose mode, we have to consume the debug output from the # child or the child will block, causing this test to hang in the diff --git a/Misc/NEWS.d/next/Library/2025-09-29-14-15-20.gh-issue-139184.dNl9O4.rst b/Misc/NEWS.d/next/Library/2025-09-29-14-15-20.gh-issue-139184.dNl9O4.rst index 7388afaa94ad48..5ede20eac39a9d 100644 --- a/Misc/NEWS.d/next/Library/2025-09-29-14-15-20.gh-issue-139184.dNl9O4.rst +++ b/Misc/NEWS.d/next/Library/2025-09-29-14-15-20.gh-issue-139184.dNl9O4.rst @@ -1 +1 @@ -Set O_CLOEXEC for master_fd when call os.forkpty +:func:``os.forkpty`` does now make the returned file descriptor non-inheritable. diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index be2b501e4fa889..6f9d75823c7544 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -9060,8 +9060,11 @@ os_forkpty_impl(PyObject *module) /* parent: release the import lock. */ PyOS_AfterFork_Parent(); /* set O_CLOEXEC on master_fd */ - if (_Py_set_inheritable(master_fd, 0, NULL) < 0) - goto error; + if (_Py_set_inheritable(master_fd, 0, NULL) < 0) { + close(master_fd); + return NULL; + } + // After PyOS_AfterFork_Parent() starts the world to avoid deadlock. if (warn_about_fork_with_threads("forkpty") < 0) return NULL; @@ -9071,10 +9074,6 @@ os_forkpty_impl(PyObject *module) } return Py_BuildValue("(Ni)", PyLong_FromPid(pid), master_fd); -error: - if (master_fd != -1) - close(master_fd); - return NULL; } #endif /* HAVE_FORKPTY */ From c968f3327e1b51703566fd98d956d5770988744c Mon Sep 17 00:00:00 2001 From: Manjusaka Date: Thu, 9 Oct 2025 01:45:34 +0800 Subject: [PATCH 5/7] fix review idea Signed-off-by: Manjusaka --- Doc/library/os.rst | 7 +++++++ Doc/library/pty.rst | 7 +++++++ Lib/test/test_pty.py | 1 - 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/Doc/library/os.rst b/Doc/library/os.rst index 8c81e1dcd070ab..2a4832f62628d2 100644 --- a/Doc/library/os.rst +++ b/Doc/library/os.rst @@ -4592,6 +4592,10 @@ written in Python, such as a mail server's external command delivery program. master end of the pseudo-terminal. For a more portable approach, use the :mod:`pty` module. If an error occurs :exc:`OSError` is raised. + The returned file descriptor *fd* is :ref:`non-inheritable `. + This means it will not be passed to child processes created by subsequent + :func:`fork` or :func:`exec*` calls. + .. audit-event:: os.forkpty "" os.forkpty .. warning:: @@ -4608,6 +4612,9 @@ written in Python, such as a mail server's external command delivery program. threads, this now raises a :exc:`DeprecationWarning`. See the longer explanation on :func:`os.fork`. + .. versionchanged:: 3.15 + The returned file descriptor is now made non-inheritable. + .. availability:: Unix, not WASI, not Android, not iOS. diff --git a/Doc/library/pty.rst b/Doc/library/pty.rst index 1a44bb13a841de..06ca435c35ac4d 100644 --- a/Doc/library/pty.rst +++ b/Doc/library/pty.rst @@ -33,9 +33,16 @@ The :mod:`pty` module defines the following functions: file descriptor connected to the child's controlling terminal (and also to the child's standard input and output). + The returned file descriptor *fd* is :ref:`non-inheritable `. + This means it will not be passed to child processes created by subsequent + :func:`os.fork` or :func:`os.exec*` calls. + .. warning:: On macOS the use of this function is unsafe when mixed with using higher-level system APIs, and that includes using :mod:`urllib.request`. + .. versionchanged:: 3.15 + The returned file descriptor is now made non-inheritable. + .. function:: openpty() diff --git a/Lib/test/test_pty.py b/Lib/test/test_pty.py index 966e19b9d65a50..a2018e864445e1 100644 --- a/Lib/test/test_pty.py +++ b/Lib/test/test_pty.py @@ -12,7 +12,6 @@ raise unittest.SkipTest("pty is not available on this platform") import errno -import fcntl import os import pty import tty From f78e916e409fa7d2d2e001ec6eb21c54f8c90a80 Mon Sep 17 00:00:00 2001 From: Manjusaka Date: Thu, 9 Oct 2025 02:03:43 +0800 Subject: [PATCH 6/7] fix review idea Signed-off-by: Manjusaka --- .../next/Library/2025-09-29-14-15-20.gh-issue-139184.dNl9O4.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Library/2025-09-29-14-15-20.gh-issue-139184.dNl9O4.rst b/Misc/NEWS.d/next/Library/2025-09-29-14-15-20.gh-issue-139184.dNl9O4.rst index 5ede20eac39a9d..d50cdeaadc45a6 100644 --- a/Misc/NEWS.d/next/Library/2025-09-29-14-15-20.gh-issue-139184.dNl9O4.rst +++ b/Misc/NEWS.d/next/Library/2025-09-29-14-15-20.gh-issue-139184.dNl9O4.rst @@ -1 +1 @@ -:func:``os.forkpty`` does now make the returned file descriptor non-inheritable. +:func:`os.forkpty` does now make the returned file descriptor non-inheritable. From a2aa47bc3aeee937698eb225bb34fd9e81449bf6 Mon Sep 17 00:00:00 2001 From: Manjusaka Date: Thu, 9 Oct 2025 02:19:42 +0800 Subject: [PATCH 7/7] fix docs Signed-off-by: Manjusaka --- Doc/library/os.rst | 2 +- Doc/library/pty.rst | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Doc/library/os.rst b/Doc/library/os.rst index 2a4832f62628d2..3e2b3f9f350e91 100644 --- a/Doc/library/os.rst +++ b/Doc/library/os.rst @@ -4594,7 +4594,7 @@ written in Python, such as a mail server's external command delivery program. The returned file descriptor *fd* is :ref:`non-inheritable `. This means it will not be passed to child processes created by subsequent - :func:`fork` or :func:`exec*` calls. + :func:`fork` or :func:`exec\* ` calls. .. audit-event:: os.forkpty "" os.forkpty diff --git a/Doc/library/pty.rst b/Doc/library/pty.rst index 06ca435c35ac4d..416a1ed1263fe7 100644 --- a/Doc/library/pty.rst +++ b/Doc/library/pty.rst @@ -35,7 +35,7 @@ The :mod:`pty` module defines the following functions: The returned file descriptor *fd* is :ref:`non-inheritable `. This means it will not be passed to child processes created by subsequent - :func:`os.fork` or :func:`os.exec*` calls. + :func:`os.fork` or :func:`os.exec\* ` calls. .. warning:: On macOS the use of this function is unsafe when mixed with using higher-level system APIs, and that includes using :mod:`urllib.request`.