[issue4841] io's close() not handling errors correctly

STINNER Victor report at bugs.python.org
Wed Jan 7 13:38:53 CET 2009


STINNER Victor <victor.stinner at haypocalc.com> added the comment:

> 1. The handling of the error in internal_close() isn't really good. What
> I'm also not sure about is whether it is correct to set the error there,
> the old code sometimes did set an error as reaction to internal_close()
> failing and sometimes not.

internal_close() is used in:
 - fileio_close(): raise an IOError(errno) on internal_close() failure
 - dircheck(): ignore internal_close() failure, whereas Python 3.x raise 
   a IOError(errno) on internal_close() failure
 - file_init(): catch internal_close() failure but don't set an exception,
   whereas Python 3.x set a IOError(errno)
 - fileio_dealloc(): PySys_WriteStderr(...) in Python 2.x,
   PyErr_WriteUnraisable(self) in Python 3.x

But Python 2.x all errors are ignored because the test (errno < 0) is wrong...

I always prefer Python 3.x behaviour: internal_close() is responsible to raise 
an exception.

> 2. There is one place in dircheck() where I think that the error should
> be ignored, but I'm not sure about that.

I rarely a good idea to ignore errors, I prefer to raise a different error if 
the file can not be closed.

> 3. This patch fixes another issue, and that is that internal_close()
> would call close() even if 'closefd' was not set. This e.g. happens when
> __init__ is called explicitly. This case is missing a test though.

Python 3.x is also affected by this issue: file_init() and dircheck() ignore 
self->closefd.

passwd = open("/etc/passwd")
f = _FileIO(fd, closefd=False)
f.__init__(fd, closefd=False) => close passwd file!

> 4. I wonder if internal_close() should reset only the 'fd' field or if
> it should also reset 'readable', 'writable', 'seekable', too, i.e. apart
> from 'weakreflist' do the same as fileio_new() does.

seekable(), readable() and writable() raise an error if the file is closed. I 
think it's fine to leave the internal attributes unchanged.

_______________________________________
Python tracker <report at bugs.python.org>
<http://bugs.python.org/issue4841>
_______________________________________


More information about the Python-bugs-list mailing list