Repository: pypa/setuptools (python)

Issue:
[Errno 17] File exists when extracting files to egg cache (or creating egg cache directory)
We are experiencing intermittent failures when running tests for one of our Python applications while installing dependencies during test execution in our CI system. The tests are being run with pytest, and we are using the pytest-xdist plugin to run tests in multiple processes in parallel to reduce the execution time.

Here is an example stack trace:
```
.eggs/py-1.5.2-py2.7.egg/py/_path/common.py:377: in visit
    for x in Visitor(fil, rec, ignore, bf, sort).gen(self):
.eggs/py-1.5.2-py2.7.egg/py/_path/common.py:429: in gen
    for p in self.gen(subdir):
.eggs/py-1.5.2-py2.7.egg/py/_path/common.py:419: in gen
    if p.check(dir=1) and (rec is None or rec(p))])
.eggs/pytest-3.2.5-py2.7.egg/_pytest/main.py:737: in _recurse
    ihook = self.gethookproxy(path)
.eggs/pytest-3.2.5-py2.7.egg/_pytest/main.py:641: in gethookproxy
    my_conftestmodules = pm._getconftestmodules(fspath)
.eggs/pytest-3.2.5-py2.7.egg/_pytest/config.py:356: in _getconftestmodules
    mod = self._importconftest(conftestpath)
.eggs/pytest-3.2.5-py2.7.egg/_pytest/config.py:381: in _importconftest
    raise ConftestImportFailure(conftestpath, sys.exc_info())
E   ConftestImportFailure: ExtractionError("Can't extract file(s) to egg cache\n\nThe following error occurred while trying to extract file(s)\nto the Python egg cache:\n\n  [Errno 17] File exists: '/home/travis/.cache/Python-Eggs'\n\nThe Python egg cache directory is currently set to:\n\n  /home/travis/.cache/Python-Eggs\n\nPerhaps your account does not have write access to this directory?\nYou can change the cache directory by setting the PYTHON_EGG_CACHE\nenvironment variable to point to an accessible directory.\n",)
E     File "/home/travis/build/finn/fiaas-deploy-daemon/.eggs/pytest-3.2.5-py2.7.egg/_pytest/assertion/rewrite.py", line 212, in load_module
E       py.builtin.exec_(co, mod.__dict__)
E     File "/home/travis/build/finn/fiaas-deploy-daemon/.eggs/py-1.5.2-py2.7.egg/py/_builtin.py", line 221, in exec_
E       exec2(obj, globals, locals)
E     File "<string>", line 7, in exec2
E     File "/home/travis/build/finn/fiaas-deploy-daemon/tests/fiaas_deploy_daemon/conftest.py", line 9, in <module>
E       from fiaas_deploy_daemon.specs.models import AppSpec, ResourceRequirementSpec, ResourcesSpec, PrometheusSpec, \
E     File "/home/travis/build/finn/fiaas-deploy-daemon/fiaas_deploy_daemon/__init__.py", line 13, in <module>
E       from .crd import CustomResourceDefinitionBindings, DisabledCustomResourceDefinitionBindings
E     File "/home/travis/build/finn/fiaas-deploy-daemon/fiaas_deploy_daemon/crd/__init__.py", line 7, in <module>
E       from .watcher import CrdWatcher
E     File "/home/travis/build/finn/fiaas-deploy-daemon/fiaas_deploy_daemon/crd/watcher.py", line 12, in <module>
E       from ..deployer import DeployerEvent
E     File "/home/travis/build/finn/fiaas-deploy-daemon/fiaas_deploy_daemon/deployer/__init__.py", line 8, in <module>
E       from .deploy import Deployer
E     File "/home/travis/build/finn/fiaas-deploy-daemon/fiaas_deploy_daemon/deployer/deploy.py", line 7, in <module>
E       from .kubernetes.ready_check import ReadyCheck
E     File "/home/travis/build/finn/fiaas-deploy-daemon/fiaas_deploy_daemon/deployer/kubernetes/__init__.py", line 7, in <module>
E       from .adapter import K8s
E     File "/home/travis/build/finn/fiaas-deploy-daemon/fiaas_deploy_daemon/deployer/kubernetes/adapter.py", line 9, in <module>
E       from ...specs.models import ResourcesSpec, ResourceRequirementSpec
E     File "/home/travis/build/finn/fiaas-deploy-daemon/fiaas_deploy_daemon/specs/__init__.py", line 10, in <module>
E       from .app_config_downloader import AppConfigDownloader
E     File "/home/travis/build/finn/fiaas-deploy-daemon/fiaas_deploy_daemon/specs/app_config_downloader.py", line 1, in <module>
E       import yaml
E     File "build/bdist.linux-x86_64/egg/yaml/__init__.py", line 14, in <module>
E     File "build/bdist.linux-x86_64/egg/yaml/cyaml.py", line 5, in <module>
E     File "build/bdist.linux-x86_64/egg/_yaml.py", line 7, in <module>
E     File "build/bdist.linux-x86_64/egg/_yaml.py", line 4, in __bootstrap__
E     File "/home/travis/.local/share/virtualenvs/fiaas-deploy-daemon/lib/python2.7/site-packages/pkg_resources/__init__.py", line 1226, in resource_filename
E       self, resource_name
E     File "/home/travis/.local/share/virtualenvs/fiaas-deploy-daemon/lib/python2.7/site-packages/pkg_resources/__init__.py", line 1726, in get_resource_filename
E       self._extract_resource(manager, self._eager_to_zip(name))
E     File "/home/travis/.local/share/virtualenvs/fiaas-deploy-daemon/lib/python2.7/site-packages/pkg_resources/__init__.py", line 1756, in _extract_resource
E       self.egg_name, self._parts(zip_path)
E     File "/home/travis/.local/share/virtualenvs/fiaas-deploy-daemon/lib/python2.7/site-packages/pkg_resources/__init__.py", line 1293, in get_cache_path
E       self.extraction_error()
E     File "/home/travis/.local/share/virtualenvs/fiaas-deploy-daemon/lib/python2.7/site-packages/pkg_resources/__init__.py", line 1273, in extraction_error
E       raise err
```

While I'm not that familiar with the inner workings of pkg_resources, I took a look at the code referenced at the end of the stack trace, and my theory is that since the tests run in parallel, multiple threads may end up calling [_bypass_ensure_directory](https://github.com/pypa/setuptools/blob/master/pkg_resources/__init__.py#L3075-L3082) at the same time, and that the error stems from a file system race condition between `isdir` and `mkdir` when creating the directory, similar to in https://github.com/pypa/setuptools/pull/1083. Perhaps using the same solution could work here?

Pull request:
Egg race condition
## Summary of changes
This PR is an approach to #1412 that adds a test to induce this race condition. The test relies on implementation details, but that is made explicit in the text of the test, I have done it this way because there isn't another great way to deterministically induce the problem.

cc: @JonKohler

Closes #1256

### Pull Request Checklist
- [x] Changes have tests
- [x] News fragment added in changelog.d. See [documentation](http://setuptools.readthedocs.io/en/latest/developer-guide.html#making-a-pull-request) for details


Patch:
diff --git a/changelog.d/1418.change.rst b/changelog.d/1418.change.rst
new file mode 100644
index 000000000..d7656f574
--- /dev/null
+++ b/changelog.d/1418.change.rst
@@ -0,0 +1,1 @@
+Solved race in when creating egg cache directories.
diff --git a/pkg_resources/__init__.py b/pkg_resources/__init__.py
index 670154081..4f42156d0 100644
--- a/pkg_resources/__init__.py
+++ b/pkg_resources/__init__.py
@@ -47,6 +47,11 @@ except ImportError:
     # Python 3.2 compatibility
     import imp as _imp
 
+try:
+    FileExistsError
+except NameError:
+    FileExistsError = OSError
+
 from pkg_resources.extern import six
 from pkg_resources.extern.six.moves import urllib, map, filter
 
@@ -3030,7 +3035,10 @@ def _bypass_ensure_directory(path):
     dirname, filename = split(path)
     if dirname and filename and not isdir(dirname):
         _bypass_ensure_directory(dirname)
-        mkdir(dirname, 0o755)
+        try:
+            mkdir(dirname, 0o755)
+        except FileExistsError:
+            pass
 
 
 def split_sections(s):
diff --git a/setuptools/command/egg_info.py b/setuptools/command/egg_info.py
index 5fd6c8880..f3e604d3f 100755
--- a/setuptools/command/egg_info.py
+++ b/setuptools/command/egg_info.py
@@ -116,33 +116,7 @@ def translate_pattern(glob):
     return re.compile(pat, flags=re.MULTILINE|re.DOTALL)
 
 
-class InfoCommon:
-    tag_build = None
-    tag_date = None
-
-    @property
-    def name(self):
-        return safe_name(self.distribution.get_name())
-
-    def tagged_version(self):
-        version = self.distribution.get_version()
-        # egg_info may be called more than once for a distribution,
-        # in which case the version string already contains all tags.
-        if self.vtags and version.endswith(self.vtags):
-            return safe_version(version)
-        return safe_version(version + self.vtags)
-
-    def tags(self):
-        version = ''
-        if self.tag_build:
-            version += self.tag_build
-        if self.tag_date:
-            version += time.strftime("-%Y%m%d")
-        return version
-    vtags = property(tags)
-
-
-class egg_info(InfoCommon, Command):
+class egg_info(Command):
     description = "create a distribution's .egg-info directory"
 
     user_options = [
@@ -159,9 +133,14 @@ class egg_info(InfoCommon, Command):
     }
 
     def initialize_options(self):
+        self.egg_name = None
+        self.egg_version = None
         self.egg_base = None
         self.egg_info = None
+        self.tag_build = None
+        self.tag_date = 0
         self.broken_egg_info = False
+        self.vtags = None
 
     ####################################
     # allow the 'tag_svn_revision' to be detected and
@@ -188,15 +167,11 @@ class egg_info(InfoCommon, Command):
         egg_info['tag_date'] = 0
         edit_config(filename, dict(egg_info=egg_info))
 
-    @property
-    def egg_name(self):
-        return self.name
-
-    @property
-    def egg_version(self):
-        return self.tagged_version()
-
     def finalize_options(self):
+        self.egg_name = safe_name(self.distribution.get_name())
+        self.vtags = self.tags()
+        self.egg_version = self.tagged_version()
+
         parsed_version = parse_version(self.egg_version)
 
         try:
@@ -279,6 +254,14 @@ class egg_info(InfoCommon, Command):
         if not self.dry_run:
             os.unlink(filename)
 
+    def tagged_version(self):
+        version = self.distribution.get_version()
+        # egg_info may be called more than once for a distribution,
+        # in which case the version string already contains all tags.
+        if self.vtags and version.endswith(self.vtags):
+            return safe_version(version)
+        return safe_version(version + self.vtags)
+
     def run(self):
         self.mkpath(self.egg_info)
         installer = self.distribution.fetch_build_egg
@@ -294,6 +277,14 @@ class egg_info(InfoCommon, Command):
 
         self.find_sources()
 
+    def tags(self):
+        version = ''
+        if self.tag_build:
+            version += self.tag_build
+        if self.tag_date:
+            version += time.strftime("-%Y%m%d")
+        return version
+
     def find_sources(self):
         """Generate SOURCES.txt manifest file"""
         manifest_filename = os.path.join(self.egg_info, "SOURCES.txt")


Test patch:
diff --git a/pkg_resources/tests/test_pkg_resources.py b/pkg_resources/tests/test_pkg_resources.py
index 4e2cac945..62a39b8fa 100644
--- a/pkg_resources/tests/test_pkg_resources.py
+++ b/pkg_resources/tests/test_pkg_resources.py
@@ -12,6 +12,11 @@ import stat
 import distutils.dist
 import distutils.command.install_egg_info
 
+try:
+    from unittest import mock
+except ImportError:
+    import mock
+
 from pkg_resources.extern.six.moves import map
 from pkg_resources.extern.six import text_type, string_types
 
@@ -138,8 +143,34 @@ class TestResourceManager:
         message = "Unexpected type from get_cache_path: " + type_
         assert isinstance(path, string_types), message
 
+    def test_get_cache_path_race(self, tmpdir):
+        # Patch to os.path.isdir to create a race condition
+        def patched_isdir(dirname, unpatched_isdir=pkg_resources.isdir):
+            patched_isdir.dirnames.append(dirname)
+
+            was_dir = unpatched_isdir(dirname)
+            if not was_dir:
+                os.makedirs(dirname)
+            return was_dir
+
+        patched_isdir.dirnames = []
+
+        # Get a cache path with a "race condition"
+        mgr = pkg_resources.ResourceManager()
+        mgr.set_extraction_path(str(tmpdir))
+
+        archive_name = os.sep.join(('foo', 'bar', 'baz'))
+        with mock.patch.object(pkg_resources, 'isdir', new=patched_isdir):
+            mgr.get_cache_path(archive_name)
+
+        # Because this test relies on the implementation details of this
+        # function, these assertions are a sentinel to ensure that the
+        # test suite will not fail silently if the implementation changes.
+        called_dirnames = patched_isdir.dirnames
+        assert len(called_dirnames) == 2
+        assert called_dirnames[0].split(os.sep)[-2:] == ['foo', 'bar']
+        assert called_dirnames[1].split(os.sep)[-1:] == ['foo']
 
-class TestIndependence:
     """
     Tests to ensure that pkg_resources runs independently from setuptools.
     """