New submission from Robert Xiao <nneon...@gmail.com>:

tempfile.NamedTemporaryFile creates its wrapper like so:

    try:
        file = _io.open(fd, mode, buffering=buffering,
                        newline=newline, encoding=encoding, errors=errors)

        return _TemporaryFileWrapper(file, name, delete)
    except BaseException:
        _os.unlink(name)
        _os.close(fd)
        raise

If _TemporaryFileWrapper throws any kind of exception (even KeyboardInterrupt), 
this closes `fd` but leaks a valid `file` pointing to that fd. The `file` will 
later attempt to close the `fd` when it is collected, which can lead to subtle 
bugs. (This particular issue contributed to this bug: 
https://nedbatchelder.com/blog/202001/bug_915_please_help.html)

This should probably be rewritten as:

    try:
        file = _io.open(fd, mode, buffering=buffering,
                        newline=newline, encoding=encoding, errors=errors)
    except:
        _os.unlink(name)
        _os.close(fd)
        raise

    try:
        return _TemporaryFileWrapper(file, name, delete)
    except BaseException:
        _os.unlink(name)
        file.close()
        raise

or perhaps use nested try blocks to avoid the _os.unlink duplication.

----------
components: Library (Lib)
messages: 359888
nosy: nneonneo
priority: normal
severity: normal
status: open
title: NamedTemporaryFile could cause double-close on an fd if 
_TemporaryFileWrapper throws
type: behavior
versions: Python 3.5, Python 3.6, Python 3.7, Python 3.8, Python 3.9

_______________________________________
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

Reply via email to