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

[SilentGhost]

Changes by SilentGhost :


--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Roundup Robot]

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: Cleanup NamedTemporaryFile if open() fails, by SilentGhost
https://hg.python.org/cpython/rev/a1c125f21db4

New changeset 865cf8eba51a by Martin Panter in branch 'default':
Issue #26385: Merge NamedTemporaryFile fix from 3.5
https://hg.python.org/cpython/rev/865cf8eba51a

--
nosy: +python-dev

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Martin Panter]

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 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[SilentGhost]

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Martin Panter]

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).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[SilentGhost]

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 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Eryk Sun]

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 with delete sharing. But I see what you meant now. Yes, the 
order needs to be reversed as unlink() and then close() for this to work. Doing 
the close first does raise a FileNotFoundError.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Terry J. Reedy]

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 over-broad.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Martin Panter]

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 sounds like it would be better to call unlink() before 
close().

Terry & Victor: Writing the explicit “except BaseException:” makes it clear you 
weren’t being lazy in figuring out what exceptions you want to catch. But in 
this case the “raise” at the end of the the exception handler make it clear 
enough for me. I would be happy with either option.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[STINNER Victor]

STINNER Victor added the comment:

I prefer "except:" over "except BaseException:". What is the benefit
of passing explicitly BaseException?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Terry J. Reedy]

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.

--
nosy: +terry.reedy

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Georg Brandl]

Changes by Georg Brandl :


--
nosy:  -georg.brandl

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[STINNER Victor]

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 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Martin Panter]

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 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[STINNER Victor]

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[SilentGhost]

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

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Eryk Sun]

Eryk Sun added the comment:

> Does the unlink() work on Windows?

Yes. O_TEMPORARY opens the file with FILE_SHARE_DELETE, so unlink won't raise 
an error. 

Opening a file creates and returns a handle for a kernel File object that 
references the underlying file/link/stream control block in the file system. 
There may be multiple open File objects from separate NtCreateFile and 
NtOpenFile system calls, but they all reference a common file control block. 
Deleting a file requires a File handle, which is used to set the delete 
disposition in the control block. When all references (handle and pointer) to 
all File objects that reference the file are closed, the file is unlinked if 
the delete disposition is set.

The way delete-on-close works is to set a flag in the File object that causes 
the delete disposition to be automatically set when the File object is closed. 
However, any File handle that references the file can be used to set or unset 
this disposition if it has DELETE access. So it's harmless to manually call 
DeleteFile on the file (i.e. NtOpenFile and NtSetInformationFile to set the 
delete disposition) beforehand.

--
nosy: +eryksun

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Martin Panter]

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 review.

This problem also affects Python 2, if you fudge the right wrong parameters:

>>> NamedTemporaryFile((), prefix="blaua.")
Traceback (most recent call last):
  File "", line 1, in 
  File "/home/proj/python/cpython/Lib/tempfile.py", line 477, in 
NamedTemporaryFile
file = _os.fdopen(fd, mode, bufsize)
TypeError: argument 2 must be string, not tuple
[59140 refs]
>>> glob("/tmp/blaua.*")
['/tmp/blaua.AFtEqx']

--
nosy: +martin.panter
versions: +Python 2.7 -Python 3.4

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[SilentGhost]

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

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

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

[Eugene Viktorov]

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
messages: 260466
nosy: Eugene Viktorov
priority: normal
severity: normal
status: open
title: the call of tempfile.NamedTemporaryFile fails and leaves a file on the 
disk
type: behavior
versions: Python 3.4
Added file: http://bugs.python.org/file41957/temp_file.py

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com