Hacker Newsnew | past | comments | ask | show | jobs | submitlogin

> I did not see where the exception caused the issue in this case

    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
The except clause was built under the assumption that only opening the fd would fail, and thus the cleanup it performs is in those terms, especially the part about closing the file descriptor and ignoring the file object (since that's assumed not to possibly exist).

However due to the mock it's the wrapper creation which fails. This means the cleanup runs, closes the fd, but leaves the file object (created by io.open) around and assuming it owns an fd, the file object later on gets collected by the GC and closes the fd a second time, breaking anyone unlucky enough to have gotten its reuse.

This issue would not happen if the wrapper were created outside the `try` statement (though the file would be leaking to be eventually closed by the GC which is not great either)



Thanks for spending the time explaining the problematic code.




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: