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,21 @@ 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) + try: + MyFileIO(fd) + except MyException: + pass + # exception set is cleared here. MyFileIO's dealloc is called + self.assertIsNone(os.close(fd)) # should not raise OSError(EBADF) + def test_main(): # Historically, these tests have been sloppy about removing TESTFN. 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);