[issue26385] the call of tempfile.NamedTemporaryFile fails and leaves a file on the disk

Changes by SilentGhost : -- resolution: -> fixed stage: patch review -> resolved status: open -> closed ___ Python tracker ___

[issue26385] the call of tempfile.NamedTemporaryFile fails and leaves a file on the disk

Roundup Robot added the comment: New changeset 5bfb4147405e by Martin Panter in branch '2.7': Issue #26385: Cleanup NamedTemporaryFile if fdopen() fails, by SilentGhost https://hg.python.org/cpython/rev/5bfb4147405e New changeset a1c125f21db4 by Martin Panter in branch '3.5': Issue #26385:

[issue26385] the call of tempfile.NamedTemporaryFile fails and leaves a file on the disk

Martin Panter added the comment: Here is my proposed version for Python 2. -- Added file: http://bugs.python.org/file42015/issue26385_4_py2.diff ___ Python tracker

[issue26385] the call of tempfile.NamedTemporaryFile fails and leaves a file on the disk

SilentGhost added the comment: Then, I think the TypeError check could be dropped and 'wr' replaced by an obviously wrong value, both seem fairly trivial. I don't have a working 2.7 checkout, so if anyone wants to extend the fix to that branch, they're more than welcome. --

[issue26385] the call of tempfile.NamedTemporaryFile fails and leaves a file on the disk

Martin Panter added the comment: I think patch 3 is good for Python 3, thankyou. For Python 2, the test will have to be adjusted. From memory, mode='wr' is accepted without an exception, and mode=2 triggers an early error (so we never trigger the bug). --

[issue26385] the call of tempfile.NamedTemporaryFile fails and leaves a file on the disk

SilentGhost added the comment: Here is the updated patch including fixes for except and order of deletion. -- Added file: http://bugs.python.org/file41979/issue26385_3.diff ___ Python tracker

[issue26385] the call of tempfile.NamedTemporaryFile fails and leaves a file on the disk

Eryk Sun added the comment: > By your explanation, it sounds like it would be better > to call unlink() before close(). Sorry, I was responding in general, because I thought you meant unlink would fail like it would for most open files on Windows, because the CRT normally doesn't open files

[issue26385] the call of tempfile.NamedTemporaryFile fails and leaves a file on the disk

Terry J. Reedy added the comment: It makes it clearer that you know that it will 'everything' and intend to catch do so and that you did not casually toss in 'except:', as used to be the habit of many. There are over 20 bare excepts in idlelib and I suspect many or most are unintentionally

[issue26385] the call of tempfile.NamedTemporaryFile fails and leaves a file on the disk

Martin Panter added the comment: Eryk Sun: The patch proposes to add an unlink() call after the file has been closed: except Exception: _os.close(fd) # This automatically deletes the file right? _os.unlink(name) # Won’t this raise FileNotFoundError? raise By your explanation, it

[issue26385] the call of tempfile.NamedTemporaryFile fails and leaves a file on the disk

STINNER Victor added the comment: I prefer "except:" over "except BaseException:". What is the benefit of passing explicitly BaseException? -- ___ Python tracker

[issue26385] the call of tempfile.NamedTemporaryFile fails and leaves a file on the disk

Terry J. Reedy added the comment: `except:` is equivalent to `except BaseException:`. When I introduced the former into idlelib, a couple of years ago, I was told to do the latter in a follow-up patch, so that there would be no doubt as to the intent. The same should be done here.

[issue26385] the call of tempfile.NamedTemporaryFile fails and leaves a file on the disk

Changes by Georg Brandl : -- nosy: -georg.brandl ___ Python tracker ___ ___

[issue26385] the call of tempfile.NamedTemporaryFile fails and leaves a file on the disk

STINNER Victor added the comment: > Victor: You changed “except Exception:” to bare “except:” in revision > 182f08c0dd45 in Python 2, but the Python 3 code never got such a change. Ah strange :-) Python 3 must also be fixed. -- ___ Python tracker

[issue26385] the call of tempfile.NamedTemporaryFile fails and leaves a file on the disk

Martin Panter added the comment: Victor: You changed “except Exception:” to bare “except:” in revision 182f08c0dd45 in Python 2, but the Python 3 code never got such a change. -- ___ Python tracker

[issue26385] the call of tempfile.NamedTemporaryFile fails and leaves a file on the disk

STINNER Victor added the comment: > I wonder if Victor could clarify why bare except wasn't used in the python3 > version. What do you call a "bare except"? I wrote "except Exception: ; raise", it doesn't ignore the error. I want to always call the cleanup code on error. --

[issue26385] the call of tempfile.NamedTemporaryFile fails and leaves a file on the disk

SilentGhost added the comment: I wonder if Victor could clarify why bare except wasn't used in the python3 version. Anyway, here is the updated patch testing for TypeError as well. -- nosy: +haypo Added file: http://bugs.python.org/file41968/issue26385_2.diff

[issue26385] the call of tempfile.NamedTemporaryFile fails and leaves a file on the disk

Martin Panter added the comment: This looks like an extension of Issue 21058. Does the unlink() work on Windows? It seems to me that the file is configured to remove itself on close(), therefore I expect unlink() will raise an exception of its own. Also made some suggestions in the code

[issue26385] the call of tempfile.NamedTemporaryFile fails and leaves a file on the disk

SilentGhost added the comment: Here is a naïve fix including a test. -- keywords: +patch nosy: +SilentGhost, georg.brandl stage: -> patch review versions: +Python 3.5, Python 3.6 Added file: http://bugs.python.org/file41959/issue26385.diff ___

[issue26385] the call of tempfile.NamedTemporaryFile fails and leaves a file on the disk

New submission from Eugene Viktorov: When calling the tempfile.NamedTemporaryFile with mode='wr' or with any other wrong value for "mode", it raises the ValueError and silently leaves an unknown, just created file on the file system. -- components: Library (Lib) files: temp_file.py