3.8.0: dictionary changed size during iteration iterating on importlib_metadata.distributions() #293
Comments
|
Confirmed. All our CI pipeline unittests fail due to the new version: https://gitlab.com/secml/secml/-/pipelines/277431124 This is independent of the Python version. |
|
Thanks for the report. I can't tell from the traceback what's happening, and the issue isn't happening in my CI environments that also rely on importlib_metadata 3.8 and pluggy. Moreover, when looking at #290, I don't see anything in particular where a dictionary size might be altered. There is this line, but the Can someone help distill the issue into a reproducer I can use to troubleshoot? |
|
I'm able to reproduce the issue by checking out secml and running tox -e py36. |
|
If I disable the diff --git a/importlib_metadata/__init__.py b/importlib_metadata/__init__.py
index 11b9aac..1b95c8f 100644
--- a/importlib_metadata/__init__.py
+++ b/importlib_metadata/__init__.py
@@ -648,7 +648,6 @@ class FastPath:
def mtime(self):
with contextlib.suppress(OSError):
return os.stat(self.root).st_mtime
- self.lookup.cache_clear()
@method_cache
def lookup(self, mtime):
diff --git a/importlib_metadata/_functools.py b/importlib_metadata/_functools.py
index 73f50d0..632292f 100644
--- a/importlib_metadata/_functools.py
+++ b/importlib_metadata/_functools.py
@@ -75,7 +75,7 @@ def method_cache(method, cache_wrapper=None):
def wrapper(self, *args, **kwargs):
# it's the first call, replace the method with a cached, bound method
bound_method = types.MethodType(method, self)
- cached_method = cache_wrapper(bound_method)
+ cached_method = bound_method
setattr(self, method.__name__, cached_method)
return cached_method(*args, **kwargs)
|
|
If I switch to a different method of memoization of that method, the problem remains: diff --git a/importlib_metadata/__init__.py b/importlib_metadata/__init__.py
index 11b9aac..8649630 100644
--- a/importlib_metadata/__init__.py
+++ b/importlib_metadata/__init__.py
@@ -21,7 +21,7 @@ from ._compat import (
Protocol,
)
-from ._functools import method_cache
+from ._functools import memoize
from ._itertools import unique_everseen
from configparser import ConfigParser
@@ -648,9 +648,8 @@ class FastPath:
def mtime(self):
with contextlib.suppress(OSError):
return os.stat(self.root).st_mtime
- self.lookup.cache_clear()
- @method_cache
+ @memoize
def lookup(self, mtime):
return Lookup(self)
diff --git a/importlib_metadata/_functools.py b/importlib_metadata/_functools.py
index 73f50d0..f678ce0 100644
--- a/importlib_metadata/_functools.py
+++ b/importlib_metadata/_functools.py
@@ -83,3 +83,44 @@ def method_cache(method, cache_wrapper=None):
wrapper.cache_clear = lambda: None
return wrapper
+
+
+# from
+# http://code.activestate.com/recipes/577452-a-memoize-decorator-for-instance-methods/
+class memoize:
+ """cache the return value of a method
+
+ This class is meant to be used as a decorator of methods. The return value
+ from a given method invocation will be cached on the instance whose method
+ was invoked. All arguments passed to a method decorated with memoize must
+ be hashable.
+
+ If a memoized method is invoked directly on its class the result will not
+ be cached. Instead the method will be invoked like a static method:
+ class Obj(object):
+ @memoize
+ def add_to(self, arg):
+ return self + arg
+ Obj.add_to(1) # not enough arguments
+ Obj.add_to(1, 2) # returns 3, result is not cached
+ """
+ def __init__(self, func):
+ self.func = func
+
+ def __get__(self, obj, objtype=None):
+ if obj is None:
+ return self.func
+ return functools.partial(self, obj)
+
+ def __call__(self, *args, **kw):
+ obj = args[0]
+ try:
+ cache = obj.__cache
+ except AttributeError:
+ cache = obj.__cache = {}
+ key = (self.func, args[1:], frozenset(kw.items()))
+ try:
+ res = cache[key]
+ except KeyError:
+ res = cache[key] = self.func(*args, **kw)
+ return resThat indicates that it's the memoization of the lookup object that's implicated in the failure. |
|
The only change between 3.7.3 and 3.8.0 is the caching (v3.7.3...v3.8.0). |
|
I've figured out the issue. It's in how Lookup.search uses lazy evaluation of self.info and how
|
|
Gotcha. |
|
I've got a workaround going out in 3.8.1. I still want to produce a test that captures the failure, but I wanted to get the fix out asap. |
|
I just tested your fix and it solves my problem. |
|
Sorry for the inconvenience. |
|
No worries, thanks for fixing it so quickly. |
3.8 is breaking my CI Link.
For now I will pin to 3.7.3, but you might want to investigate it.
Exception:
The text was updated successfully, but these errors were encountered: