Failure to try another name for tempfile when directory with chosen name exists on windows

[Vlad-Shcherbina]

BPO	18849
Nosy	@birkenfeld, @ncoghlan, @serhiy-storchaka, @Vlad-Shcherbina
Files	temp_dir_exists_retry_test_33_34.patch temp_dir_exists_retry_33_34.patch temp_dir_exists_retry_test_27.patch temp_dir_exists_retry_27.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2013-09-06.13:17:55.799>
created_at = <Date 2013-08-27.08:58:41.110>
labels = ['type-bug', 'library', 'OS-windows']
title = 'Failure to try another name for tempfile when directory with chosen name exists on windows'
updated_at = <Date 2013-09-06.13:17:55.798>
user = 'https://github.com/Vlad-Shcherbina'

bugs.python.org fields:

activity = <Date 2013-09-06.13:17:55.798>
actor = 'python-dev'
assignee = 'none'
closed = True
closed_date = <Date 2013-09-06.13:17:55.799>
closer = 'python-dev'
components = ['Library (Lib)', 'Windows']
creation = <Date 2013-08-27.08:58:41.110>
creator = 'vlad'
dependencies = []
files = ['31606', '31607', '31608', '31609']
hgrepos = []
issue_num = 18849
keywords = ['patch']
message_count = 6.0
messages = ['196269', '196838', '196899', '196906', '197065', '197066']
nosy_count = 6.0
nosy_names = ['georg.brandl', 'ncoghlan', 'eli.bendersky', 'python-dev', 'serhiy.storchaka', 'vlad']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue18849'
versions = ['Python 2.7', 'Python 3.3', 'Python 3.4']

[Vlad-Shcherbina]

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'

[elibendersky]

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.

[Vlad-Shcherbina]

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

[elibendersky]

Re (1) let's leave it as it is, now. I don't think it really matters.

I left some comments in Rietveld.

[python-dev]

New changeset 7611e7244bdd by Eli Bendersky in branch '3.3':
Issue bpo-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 bpo-18849: Fixed a Windows-specific tempfile bug where collision with an
http://hg.python.org/cpython/rev/035b61b52caa

[python-dev]

New changeset e0037f266d45 by Eli Bendersky in branch '2.7':
Close bpo-18849: Fixed a Windows-specific tempfile bug where collision with an
http://hg.python.org/cpython/rev/e0037f266d45