[Python-checkins] cpython (merge 3.2 -> default): #10053: Don't close FDs when FileIO.__init__ fails

[hynek.schlawack]

http://hg.python.org/cpython/rev/464cf523485e
changeset:   77539:464cf523485e
parent:      77536:890d5c8dc59f
parent:      77538:d042bd8625f3
user:        Hynek Schlawack <hs at ox.cx>
date:        Thu Jun 21 20:58:31 2012 +0200
summary:
  #10053: Don't close FDs when FileIO.__init__ fails

Loosely based on the work by Hirokazu Yamamoto.

files:
  Lib/test/test_fileio.py |  11 +++++++++++
  Misc/NEWS               |   3 +++
  Modules/_io/fileio.c    |  12 ++++++------
  3 files changed, 20 insertions(+), 6 deletions(-)


diff --git a/Lib/test/test_fileio.py b/Lib/test/test_fileio.py
--- a/Lib/test/test_fileio.py
+++ b/Lib/test/test_fileio.py
@@ -404,6 +404,17 @@
             self.assertRaises(ValueError, _FileIO, "/some/invalid/name", "rt")
             self.assertEqual(w.warnings, [])
 
+    def testUnclosedFDOnException(self):
+        class MyException(Exception): pass
+        class MyFileIO(_FileIO):
+            def __setattr__(self, name, value):
+                if name == "name":
+                    raise MyException("blocked setting name")
+                return super(MyFileIO, self).__setattr__(name, value)
+        fd = os.open(__file__, os.O_RDONLY)
+        self.assertRaises(MyException, MyFileIO, fd)
+        os.close(fd)  # should not raise OSError(EBADF)
+
 
 def test_main():
     # Historically, these tests have been sloppy about removing TESTFN.
diff --git a/Misc/NEWS b/Misc/NEWS
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -10,6 +10,9 @@
 Core and Builtins
 -----------------
 
+- Issue #10053: Don't close FDs when FileIO.__init__ fails. Loosely based on
+  the work by Hirokazu Yamamoto.
+
 - Issue #15096: Removed support for ur'' as the raw notation isn't
   compatible with Python 2.x's raw unicode strings.
 
diff --git a/Modules/_io/fileio.c b/Modules/_io/fileio.c
--- a/Modules/_io/fileio.c
+++ b/Modules/_io/fileio.c
@@ -227,6 +227,7 @@
     int flags = 0;
     int fd = -1;
     int closefd = 1;
+    int fd_is_own = 0;
 
     assert(PyFileIO_Check(oself));
     if (self->fd >= 0) {
@@ -376,6 +377,7 @@
 #endif
                 self->fd = open(name, flags, 0666);
             Py_END_ALLOW_THREADS
+            fd_is_own = 1;
         } else {
             PyObject *fdobj = PyObject_CallFunction(
                                   opener, "Oi", nameobj, flags);
@@ -393,6 +395,7 @@
             if (self->fd == -1) {
                 goto error;
             }
+            fd_is_own = 1;
         }
 
         if (self->fd < 0) {
@@ -421,13 +424,8 @@
            end of file (otherwise, it might be done only on the
            first write()). */
         PyObject *pos = portable_lseek(self->fd, NULL, 2);
-        if (pos == NULL) {
-            if (closefd) {
-                close(self->fd);
-                self->fd = -1;
-            }
+        if (pos == NULL)
             goto error;
-        }
         Py_DECREF(pos);
     }
 
@@ -435,6 +433,8 @@
 
  error:
     ret = -1;
+    if (!fd_is_own)
+        self->fd = -1;
     if (self->fd >= 0)
         internal_close(self);
 

-- 
Repository URL: http://hg.python.org/cpython