Paul Ollis <pyt...@ollis.me.uk> added the comment:
I think it is worth pointing out that the semantics of
f = ``open(fd, closefd=True)``
are broken (IMHO) because an exception can result in an unreferenced file
object that has taken over reponsibility for closing the fd, but it can
also fail without creating the file object.
Therefore it should be considered a bad idea to use open in this way. So
NamedTemporaryFile should take responsibility for closing the fd; i.e. it
should used closefd=False.
I would suggest that NamedTemporaryFile's code should be:
try:
file = _io.open(fd, mode, buffering=buffering, closefd=False,
newline=newline, encoding=encoding, errors=errors)
return _TemporaryFileWrapper(file, name, delete)
except BaseException:
_os.close(fd)
try:
_os.unlink(name)
except OSError:
pass # On windows the file will already be deleted.
raise
And '_os.close(self.file.fileno())' should be added after each of the two calls
to 'self.file.close()' in _TemporaryFileCloser.
Some notes on the design choices here.
1. The exception handling performs the close *before* the unlink because:
- That is the normal order that people expect.
- It is most important that the file is closed. We cannot rule out the
possibility of the unlink raising an exception, which could leak the fd.
2. BaseException is caught because we should not leak a file descriptor for
KeyboardInterrupt or any other exception.
3. It is generally good practice to protect os.unlink with a try ... except
OSError. So I think this is an appropriate way to prevent the Windows
specific error.
It will also suppress some other, rare but possible, errors. I think that
this is legitimate behaviour, but it should be documented.
----------
nosy: +paul_ollis
_______________________________________
Python tracker <rep...@bugs.python.org>
<https://bugs.python.org/issue39318>
_______________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe:
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
