make sure fdopen always closes the fd in error cases (closes #21191)

diff --git a/Doc/library/os.rst b/Doc/library/os.rst
index a597a67d39dc8eba2b170c4c71bb8da930225da5..195a4a80fcb83b83411248bc3ede35581a820f95 100644 (file)
--- a/Doc/library/os.rst
+++ b/Doc/library/os.rst
@@ -463,8 +463,9 @@ These functions create new file objects. (See also :func:`open`.)
    .. index:: single: I/O control; buffering
 
    Return an open file object connected to the file descriptor *fd*.  The *mode*
-   and *bufsize* arguments have the same meaning as the corresponding arguments to
-   the built-in :func:`open` function.
+   and *bufsize* arguments have the same meaning as the corresponding arguments
+   to the built-in :func:`open` function.  If :func:`fdopen` raises an
+   exception, it closes *fd*.
 
    Availability: Unix, Windows.
 

diff --git a/Lib/test/test_posix.py b/Lib/test/test_posix.py
index 76a74fb907ba2b4ad24b3e17e03442ee54a4d0f1..243fd5d6ed13729a573e6ddf98e45d8661132381 100644 (file)
--- a/Lib/test/test_posix.py
+++ b/Lib/test/test_posix.py
@@ -194,6 +194,10 @@ class PosixTester(unittest.TestCase):
         self.fdopen_helper('r')
         self.fdopen_helper('r', 100)
 
+        fd = os.open(test_support.TESTFN, os.O_RDONLY)
+        self.assertRaises(OSError, posix.fdopen, fd, 'w')
+        self.assertRaises(OSError, os.close, fd) # fd should be closed.
+
     @unittest.skipUnless(hasattr(posix, 'O_EXLOCK'),
                          'test needs posix.O_EXLOCK')
     def test_osexlock(self):

diff --git a/Misc/NEWS b/Misc/NEWS
index a6dfc7e752da3476bd6ad79c540fa6a5aa1554d4..873c7b6a5d3abff39e93975167a93bd5f29c8e6e 100644 (file)
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -43,6 +43,9 @@ Core and Builtins
 Library
 -------
 
+- Issue #21191: In os.fdopen, alwyas close the file descriptor when an exception
+  happens.
+
 - Issue #21149: Improved thread-safety in logging cleanup during interpreter
   shutdown. Thanks to Devin Jeanpierre for the patch.
 

diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c
index 21a6cef3f7903da301bcac3c0ac016fed8f358c2..168f7f491731acaf86abf6979d1c6da910751948 100644 (file)
--- a/Modules/posixmodule.c
+++ b/Modules/posixmodule.c
@@ -6841,16 +6841,21 @@ posix_fdopen(PyObject *self, PyObject *args)
     /* Sanitize mode.  See fileobject.c */
     mode = PyMem_MALLOC(strlen(orgmode)+3);
     if (!mode) {
+        close(fd);
         PyErr_NoMemory();
         return NULL;
     }
     strcpy(mode, orgmode);
     if (_PyFile_SanitizeMode(mode)) {
+        close(fd);
         PyMem_FREE(mode);
         return NULL;
     }
-    if (!_PyVerify_fd(fd))
-        return posix_error();
+    if (!_PyVerify_fd(fd)) {
+        posix_error();
+        close(fd);
+        return NULL;
+    }
     Py_BEGIN_ALLOW_THREADS
 #if !defined(MS_WINDOWS) && defined(HAVE_FCNTL_H)
     if (mode[0] == 'a') {
@@ -6871,8 +6876,11 @@ posix_fdopen(PyObject *self, PyObject *args)
 #endif
     Py_END_ALLOW_THREADS
     PyMem_FREE(mode);
-    if (fp == NULL)
-        return posix_error();
+    if (fp == NULL) {
+        posix_error();
+        close(fd);
+        return NULL;
+    }
     /* The dummy filename used here must be kept in sync with the value
        tested against in gzip.GzipFile.__init__() - see issue #13781. */
     f = PyFile_FromFile(fp, "<fdopen>", orgmode, fclose);