diff --git a/Doc/library/os.rst b/Doc/library/os.rst index 3f0b91af4d0a86..da97ec3e0a7c25 100644 --- a/Doc/library/os.rst +++ b/Doc/library/os.rst @@ -1081,6 +1081,9 @@ as internal buffering of data. ` by default or non-inheritable if *inheritable* is ``False``. + If *fd* is equal to *fd2* and *inheritable* is ``True``, the "inheritable" + flag of the specified file descriptor is not changed. + .. availability:: not WASI. .. versionchanged:: 3.4 diff --git a/Lib/test/test_os/test_os.py b/Lib/test/test_os/test_os.py index 81b3043eb7e75b..e517a83223f3d8 100644 --- a/Lib/test/test_os/test_os.py +++ b/Lib/test/test_os/test_os.py @@ -2894,7 +2894,7 @@ def test_dup2_negative_fd(self): -2**31, ] for fd, fd2 in itertools.product(fds, repeat=2): - if fd != fd2: + if fd != valid_fd or fd2 != valid_fd: with self.subTest(fd=fd, fd2=fd2): with self.assertRaises(OSError) as ctx: os.dup2(fd, fd2) @@ -4743,6 +4743,27 @@ def test_dup2(self): self.assertEqual(os.dup2(fd, fd3, inheritable=False), fd3) self.assertFalse(os.get_inheritable(fd3)) + @unittest.skipUnless(hasattr(os, 'dup2'), "need os.dup2()") + @unittest.skipIf(hasattr(sys, 'getandroidapilevel') and + sys.getandroidapilevel() < 23, + "bpo-26935: os.dup2() fails for equal fds on old Android") + def test_dup2_eq_fd(self): + fd = os.open(__file__, os.O_RDONLY) + self.addCleanup(os.close, fd) + + # Non-inheritability is preserved by default and even with explicit + # inheritable=True. + os.dup2(fd, fd) + self.assertFalse(os.get_inheritable(fd)) + os.dup2(fd, fd, inheritable=True) + self.assertFalse(os.get_inheritable(fd)) + + # gh-77043: Inheritability is removed with inheritable=False. + os.set_inheritable(fd, True) + os.dup2(fd, fd, inheritable=False) + self.assertFalse(os.get_inheritable(fd)) + + @unittest.skipUnless(hasattr(os, 'openpty'), "need os.openpty()") class PseudoterminalTests(unittest.TestCase): def open_pty(self): diff --git a/Misc/NEWS.d/next/Library/2023-02-22-15-20-55.gh-issue-77043.fXztgY.rst b/Misc/NEWS.d/next/Library/2023-02-22-15-20-55.gh-issue-77043.fXztgY.rst new file mode 100644 index 00000000000000..3c84de4fc78f63 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2023-02-22-15-20-55.gh-issue-77043.fXztgY.rst @@ -0,0 +1 @@ +Make :func:`os.dup2(fd, fd, False)` always remove fd inheritability. diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index a619de79ca73b2..73db1316b0b9d3 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -11985,7 +11985,7 @@ static int os_dup2_impl(PyObject *module, int fd, int fd2, int inheritable) /*[clinic end generated code: output=bc059d34a73404d1 input=c3cddda8922b038d]*/ { - int res = 0; + int res = -1; /* dup3() is available on Linux 2.6.27+ and glibc 2.9 and macOS 27.0; * it needs runtime detection for the case of running on older kernels. @@ -11997,86 +11997,71 @@ os_dup2_impl(PyObject *module, int fd, int fd2, int inheritable) static int dup3_works_atomic = -1; (void) dup3_works_atomic; // unused on some platforms - /* dup2() can fail with EINTR if the target FD is already open, because it - * then has to be closed. See os_close_impl() for why we don't handle EINTR - * upon close(), and therefore below. - */ -#ifdef MS_WINDOWS - Py_BEGIN_ALLOW_THREADS - _Py_BEGIN_SUPPRESS_IPH - res = dup2(fd, fd2); - _Py_END_SUPPRESS_IPH - Py_END_ALLOW_THREADS - if (res < 0) { - posix_error(); - return -1; - } - res = fd2; // msvcrt dup2 returns 0 on success. - - /* Character files like console cannot be make non-inheritable */ - if (!inheritable && _Py_set_inheritable(fd2, 0, NULL) < 0) { - close(fd2); - return -1; - } - -#elif defined(HAVE_FCNTL_H) && defined(F_DUP2FD_CLOEXEC) - Py_BEGIN_ALLOW_THREADS - if (!inheritable) + // gh-77043: Always fallback to dup2() if fd == fd2 because other + // functions may behave differently in this case. + if (!inheritable && fd != fd2) { +#if defined(HAVE_FCNTL_H) && defined(F_DUP2FD_CLOEXEC) + Py_BEGIN_ALLOW_THREADS res = fcntl(fd, F_DUP2FD_CLOEXEC, fd2); - else - res = dup2(fd, fd2); - Py_END_ALLOW_THREADS - if (res < 0) { - posix_error(); - return -1; - } - -#else - -#ifdef HAVE_DUP3 - int dup3_works = FT_ATOMIC_LOAD_INT_RELAXED(dup3_works_atomic); - if (!inheritable && dup3_works != 0) { - if (HAVE_DUP3_RUNTIME) { - Py_BEGIN_ALLOW_THREADS - res = dup3(fd, fd2, O_CLOEXEC); - Py_END_ALLOW_THREADS - if (res < 0) { - if (dup3_works == -1) { - dup3_works = (errno != ENOSYS); - FT_ATOMIC_STORE_INT_RELAXED(dup3_works_atomic, dup3_works); - } - if (dup3_works) { - posix_error(); - return -1; + Py_END_ALLOW_THREADS + if (res < 0) { + posix_error(); + return -1; + } +#elif defined(HAVE_DUP3) + // dup3() is available on Linux 2.6.27+ and glibc 2.9 and macOS 27.0; + // it needs runtime detection for the case of running on older kernels. + int dup3_works = FT_ATOMIC_LOAD_INT_RELAXED(dup3_works_atomic); + if (dup3_works != 0) { + if (HAVE_DUP3_RUNTIME) { + Py_BEGIN_ALLOW_THREADS + res = dup3(fd, fd2, O_CLOEXEC); + Py_END_ALLOW_THREADS + if (res < 0) { + if (dup3_works == -1) { + dup3_works = (errno != ENOSYS); + FT_ATOMIC_STORE_INT_RELAXED(dup3_works_atomic, dup3_works); + } + if (dup3_works) { + posix_error(); + return -1; + } } } + else { + dup3_works = 0; + FT_ATOMIC_STORE_INT_RELAXED(dup3_works_atomic, dup3_works); + } } - else { - dup3_works = 0; - FT_ATOMIC_STORE_INT_RELAXED(dup3_works_atomic, dup3_works); - } +#endif } - if (inheritable || dup3_works == 0) - { -#endif + if (res < 0) { + // dup2() can fail with EINTR if the target FD is already open, because + // it then has to be closed. See os_close_impl() for why we don't + // handle EINTR upon close(), and therefore below. Py_BEGIN_ALLOW_THREADS + _Py_BEGIN_SUPPRESS_IPH res = dup2(fd, fd2); + _Py_END_SUPPRESS_IPH Py_END_ALLOW_THREADS if (res < 0) { posix_error(); return -1; } +#ifdef MS_WINDOWS + res = fd2; // msvcrt dup2 returns 0 on success. +#endif + // On Windows, character files like console cannot be made + // non-inheritable. if (!inheritable && _Py_set_inheritable(fd2, 0, NULL) < 0) { - close(fd2); + if (fd != fd2) { + close(fd2); + } return -1; } -#ifdef HAVE_DUP3 } -#endif - -#endif return res; }