Fix hash caching race condition

- Clean up more unused code - Fixes #3289 Signed-off-by: Dan Ryan <[email protected]>
pypa · Nov 23, 2018 · 9b1b901 · 9b1b901
1 parent b03983e
commit 9b1b901
Show file tree

Hide file tree

Showing 2 changed files with 42 additions and 39 deletions.
diff --git a/news/3289.bugfix.rst b/news/3289.bugfix.rst
@@ -0,0 +1 @@
+Fixed a race condition in hash resolution for dependencies for certain dependencies with missing cache entries or fresh Pipenv installs.
diff --git a/pipenv/utils.py b/pipenv/utils.py
@@ -25,20 +25,6 @@
 
 from vistir.compat import ResourceWarning
 
-try:
-    from weakref import finalize
-except ImportError:
-    try:
-        from .vendor.backports.weakref import finalize
-    except ImportError:
-
-        class finalize(object):
-            def __init__(self, *args, **kwargs):
-                logging.warn("weakref.finalize unavailable, not cleaning...")
-
-            def detach(self):
-                return False
-
 
 logging.basicConfig(level=logging.ERROR)
 
@@ -411,40 +397,56 @@ def resolve(self):
             self.resolved_tree.update(results)
             return self.resolved_tree
 
-    def resolve_hashes(self):
-        def _should_include_hash(ireq):
-            from pipenv.vendor.vistir.compat import Path, to_native_string
-            from pipenv.vendor.vistir.path import url_to_path
+    @staticmethod
+    def _should_include_hash(ireq):
+        from pipenv.vendor.vistir.compat import Path, to_native_string
+        from pipenv.vendor.vistir.path import url_to_path
 
-            # We can only hash artifacts.
-            try:
-                if not ireq.link.is_artifact:
-                    return False
-            except AttributeError:
+        # We can only hash artifacts.
+        try:
+            if not ireq.link.is_artifact:
                 return False
+        except AttributeError:
+            return False
 
-            # But we don't want normal pypi artifcats since the normal resolver
-            # handles those
-            if is_pypi_url(ireq.link.url):
-                return False
+        # But we don't want normal pypi artifcats since the normal resolver
+        # handles those
+        if is_pypi_url(ireq.link.url):
+            return False
 
-            # We also don't want to try to hash directories as this will fail
-            # as these are editable deps and are not hashable.
-            if (ireq.link.scheme == "file" and
-                    Path(to_native_string(url_to_path(ireq.link.url))).is_dir()):
-                return False
-            return True
+        # We also don't want to try to hash directories as this will fail
+        # as these are editable deps and are not hashable.
+        if (ireq.link.scheme == "file" and
+                Path(to_native_string(url_to_path(ireq.link.url))).is_dir()):
+            return False
+        return True
 
+    def resolve_hashes(self):
         if self.results is not None:
             resolved_hashes = self.resolver.resolve_hashes(self.results)
             for ireq, ireq_hashes in resolved_hashes.items():
+                # We _ALWAYS MUST PRIORITIZE_ the inclusion of hashes from local sources
+                # PLEASE *DO NOT MODIFY THIS* TO CHECK WHETHER AN IREQ ALREADY HAS A HASH
+                # RESOLVED. The resolver will pull hashes from PyPI and only from PyPI.
+                # The entire purpose of this approach is to include missing hashes.
+                # This fixes a race condition in resolution for missing dependency caches
+                # see pypa/pipenv#3289
+                if self._should_include_hash(ireq) and (
+                    not ireq_hashes or ireq.link.scheme == "file"
+                ):
+                    if not ireq_hashes:
+                        ireq_hashes = set()
+                    ireq_hashes |= set(
+                        self.resolver.repository._hash_cache.get_hash(ireq.link)
+                    )
+                else:
+                    ireq_hashes = set(ireq_hashes)
+                # The _ONLY CASE_ where we flat out set the value is if it isn't present
+                # It's a set, so otherwise we *always* need to do a union update
                 if ireq not in self.hashes:
-                    if _should_include_hash(ireq):
-                        self.hashes[ireq] = [
-                            self.resolver.repository._hash_cache.get_hash(ireq.link)
-                        ]
-                    else:
-                        self.hashes[ireq] = ireq_hashes
+                    self.hashes[ireq] = ireq_hashes
+                else:
+                    self.hashes[ireq] |= ireq_hashes
             return self.hashes