when an exception is raised in fdopen, never close the fd (changing on my mind on #21191)

4 files changed, 30 insertions, 17 deletions

diff --git a/Doc/library/os.rst b/Doc/library/os.rst
index 195a4a8..2149ae5 100644
--- a/Doc/library/os.rst
+++ b/Doc/library/os.rst
@@ -465,7 +465,7 @@ These functions create new file objects. (See also :func:`open`.)
    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.  If :func:`fdopen` raises an
-   exception, it closes *fd*.
+   exception, it leaves *fd* untouched (unclosed).
 
    Availability: Unix, Windows.
 
diff --git a/Lib/test/test_posix.py b/Lib/test/test_posix.py
index 243fd5d..d631562 100644
--- a/Lib/test/test_posix.py
+++ b/Lib/test/test_posix.py
@@ -196,7 +196,7 @@ class PosixTester(unittest.TestCase):
 
         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.
+        os.close(fd) # fd should not be closed.
 
     @unittest.skipUnless(hasattr(posix, 'O_EXLOCK'),
                          'test needs posix.O_EXLOCK')
diff --git a/Misc/NEWS b/Misc/NEWS
index f6a2c65..94d3638 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -51,7 +51,7 @@ Library
 
 - Issue #21172: isinstance check relaxed from dict to collections.Mapping.
 
-- Issue #21191: In os.fdopen, alwyas close the file descriptor when an exception
+- Issue #21191: In os.fdopen, never close the file descriptor when an exception
   happens.
 
 - Issue #21149: Improved thread-safety in logging cleanup during interpreter
diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c
index 168f7f4..7a5ef1c 100644
--- a/Modules/posixmodule.c
+++ b/Modules/posixmodule.c
@@ -6841,19 +6841,37 @@ 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)) {
-        posix_error();
-        close(fd);
+        PyMem_FREE(mode);
+        return posix_error();
+    }
+#if defined(HAVE_FSTAT) && defined(S_IFDIR) && defined(EISDIR)
+    {
+        struct stat buf;
+        if (fstat(fd, &buf) == 0 && S_ISDIR(buf.st_mode)) {
+            PyMem_FREE(mode);
+            char *msg = strerror(EISDIR);
+            PyObject *exc = PyObject_CallFunction(PyExc_IOError, "(isO)",
+                                                  EISDIR, msg, "<fdopen>");
+            PyErr_SetObject(PyExc_IOError, exc);
+            Py_XDECREF(exc);
+            return NULL;
+        }
+    }
+#endif
+    /* 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(NULL, "<fdopen>", orgmode, fclose);
+    if (f == NULL) {
+        PyMem_FREE(mode);
         return NULL;
     }
     Py_BEGIN_ALLOW_THREADS
@@ -6876,16 +6894,11 @@ posix_fdopen(PyObject *self, PyObject *args)
 #endif
     Py_END_ALLOW_THREADS
     PyMem_FREE(mode);
-    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);
-    if (f != NULL)
-        PyFile_SetBufSize(f, bufsize);
+    if (fp == NULL)
+        return posix_error();
+    /* We now know we will succeed, so initialize the file object. */
+    ((PyFileObject *)f)->f_fp = fp;
+    PyFile_SetBufSize(f, bufsize);
     return f;
 }