classification
Title: Failure to try another name for tempfile when directory with chosen name exists on windows
Type: behavior Stage: resolved
Components: Library (Lib), Windows Versions: Python 3.4, Python 3.3, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: eli.bendersky, georg.brandl, ncoghlan, python-dev, serhiy.storchaka, vlad
Priority: normal Keywords: patch

Created on 2013-08-27 08:58 by vlad, last changed 2013-09-06 13:17 by python-dev. This issue is now closed.

Files
File name Uploaded Description Edit
temp_dir_exists_retry_test_33_34.patch vlad, 2013-09-05 12:07 review
temp_dir_exists_retry_33_34.patch vlad, 2013-09-05 12:07 review
temp_dir_exists_retry_test_27.patch vlad, 2013-09-05 12:09
temp_dir_exists_retry_27.patch vlad, 2013-09-05 12:09
Messages (6)
msg196269 - (view) Author: Vlad Shcherbina (vlad) Date: 2013-08-27 08:58
When directory exists with a name chosen for new temporary file, OSError with EACCESS errno is thrown on windows, while attempts to chose another name only happen on EEXIST errors.


To reproduce, run
--------------- 8< -----------------
import sys
import tempfile
import os

print sys.platform
print sys.version

# Mock random names to ensure collision.
tempfile._RandomNameSequence = lambda: iter(['a', 'a', 'b'])

d = tempfile.mkdtemp()
print d

try:
    print tempfile.NamedTemporaryFile().name
finally:
    os.rmdir(d)
--------------- >8 -----------------


Expected result:
win32
2.7.3 (default, Apr 10 2012, 23:24:47) [MSC v.1500 64 bit (AMD64)]
...\tmpa
...\tmpb


Actual result:
win32
2.7.3 (default, Apr 10 2012, 23:24:47) [MSC v.1500 64 bit (AMD64)]
c:\users\shcher~1\appdata\local\temp\tmpa
Traceback (most recent call last):
  File "hz.py", line 13, in <module>
    print tempfile.NamedTemporaryFile().name
  File "C:\python_27_amd64\files\lib\tempfile.py", line 454, in NamedTemporaryFile
    (fd, name) = _mkstemp_inner(dir, prefix, suffix, flags)
  File "C:\python_27_amd64\files\lib\tempfile.py", line 235, in _mkstemp_inner
    fd = _os.open(file, flags, 0600)
OSError: [Errno 13] Permission denied: 'c:\\users\\shcher~1\\appdata\\local\\temp\\tmpa'
msg196838 - (view) Author: Eli Bendersky (eli.bendersky) * (Python committer) Date: 2013-09-03 13:32
The fix looks good to me, in general. Could you create a test that goes along?

My only (minor) doubt is whether this should be generalized, in two dimensions:

1. PermissionError is mapped from both EACCES and EPERM. So to make the 2.7 patch equivalent with 3.x, EPERM should also be checked. That said, Windows documents it doesn't really use EPERM (http://msdn.microsoft.com/en-us/library/5814770t.aspx).
2. Should this be restricted to Windows? Could there be other platforms that exhibit the same behavior? On the other hand, say on Linux, EACCES should not happen in a temp dir and so it's good to re-raise it. 

For (2) I don't think doing anything is necessary at this point - the added test may help because it can fail for some strange platform at some point and then the solution should be obvious. As for (1), adding EPERM into the condition probably can't hurt and it will make Python 2.7 behave more consistently with 3.x in case of some undocumented Windows behavior.
msg196899 - (view) Author: Vlad Shcherbina (vlad) Date: 2013-09-04 10:59
1. I agree that consistency between 2.7 and 3.* have some value, but maybe it's better to take less permissive approach in 3.* instead and only retry when exception is PermissionError _and_ errno is EACCES?

2. Currently it's being reraised unless platform.name == 'nt'. Also, I added a test, so if there are problems on other platforms, they will surface (I myself only tried it on linux and windows 7 though).
msg196906 - (view) Author: Eli Bendersky (eli.bendersky) * (Python committer) Date: 2013-09-04 12:59
Re (1) let's leave it as it is, now. I don't think it really matters.

I left some comments in Rietveld.
msg197065 - (view) Author: Roundup Robot (python-dev) Date: 2013-09-06 13:14
New changeset 7611e7244bdd by Eli Bendersky in branch '3.3':
Issue #18849: Fixed a Windows-specific tempfile bug where collision with an
http://hg.python.org/cpython/rev/7611e7244bdd

New changeset 035b61b52caa by Eli Bendersky in branch 'default':
Issue #18849: Fixed a Windows-specific tempfile bug where collision with an
http://hg.python.org/cpython/rev/035b61b52caa
msg197066 - (view) Author: Roundup Robot (python-dev) Date: 2013-09-06 13:17
New changeset e0037f266d45 by Eli Bendersky in branch '2.7':
Close #18849: Fixed a Windows-specific tempfile bug where collision with an
http://hg.python.org/cpython/rev/e0037f266d45
History
Date User Action Args
2013-09-06 13:17:55python-devsetstatus: open -> closed
resolution: fixed
messages: + msg197066

stage: patch review -> resolved
2013-09-06 13:14:56python-devsetnosy: + python-dev
messages: + msg197065
2013-09-05 12:09:39vladsetfiles: + temp_dir_exists_retry_27.patch
2013-09-05 12:09:26vladsetfiles: + temp_dir_exists_retry_test_27.patch
2013-09-05 12:07:59vladsetfiles: + temp_dir_exists_retry_33_34.patch
2013-09-05 12:07:48vladsetfiles: + temp_dir_exists_retry_test_33_34.patch
2013-09-05 12:07:34vladsetfiles: - temp_dir_exists_retry_27.patch
2013-09-05 12:07:27vladsetfiles: - temp_dir_exists_retry_test_27.patch
2013-09-05 12:07:23vladsetfiles: - temp_dir_exists_retry_33_34.patch
2013-09-05 12:07:18vladsetfiles: - temp_dir_exists_retry_test_33_34.patch
2013-09-05 12:04:03vladsetfiles: + temp_dir_exists_retry_27.patch
2013-09-05 12:03:53vladsetfiles: + temp_dir_exists_retry_test_27.patch
2013-09-05 12:03:22vladsetfiles: + temp_dir_exists_retry_33_34.patch
2013-09-05 12:01:43vladsetfiles: + temp_dir_exists_retry_test_33_34.patch
2013-09-05 12:01:13vladsetfiles: - temp_dir_exists_retry_test_27.patch
2013-09-05 12:01:08vladsetfiles: - temp_dir_exists_retry_test_33_34.patch
2013-09-05 12:01:04vladsetfiles: - temp_dir_exists_retry_33_34.patch
2013-09-05 12:01:00vladsetfiles: - temp_dir_exists_retry_27.patch
2013-09-04 12:59:50eli.benderskysetmessages: + msg196906
2013-09-04 10:59:54vladsetmessages: + msg196899
2013-09-04 09:31:08vladsetfiles: + temp_dir_exists_retry_test_27.patch
2013-09-04 09:30:48vladsetfiles: + temp_dir_exists_retry_test_33_34.patch
2013-09-03 13:32:04eli.benderskysetnosy: + eli.bendersky

messages: + msg196838
stage: needs patch -> patch review
2013-09-03 09:27:23vladsetfiles: + temp_dir_exists_retry_33_34.patch
2013-09-03 08:55:25vladsetfiles: + temp_dir_exists_retry_27.patch
2013-09-03 08:52:50vladsetfiles: - fix_for_27.patch
2013-08-30 12:14:58vladsetfiles: + fix_for_27.patch
keywords: + patch
2013-08-28 07:55:25pitrousetstage: needs patch
versions: + Python 2.7, Python 3.3, Python 3.4
2013-08-27 12:03:07serhiy.storchakasetnosy: + georg.brandl, ncoghlan, serhiy.storchaka
2013-08-27 08:58:41vladcreate