Skip to content

Commit d8080c0

Browse files
authored
bpo-35780: Fix errors in lru_cache() C code (GH-11623)
1 parent adad9e6 commit d8080c0

File tree

4 files changed

+234
-89
lines changed

4 files changed

+234
-89
lines changed

Lib/functools.py

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -454,7 +454,7 @@ def __hash__(self):
454454

455455
def _make_key(args, kwds, typed,
456456
kwd_mark = (object(),),
457-
fasttypes = {int, str, frozenset, type(None)},
457+
fasttypes = {int, str},
458458
tuple=tuple, type=type, len=len):
459459
"""Make a cache key from optionally typed positional and keyword arguments
460460
@@ -510,8 +510,11 @@ def lru_cache(maxsize=128, typed=False):
510510

511511
# Early detection of an erroneous call to @lru_cache without any arguments
512512
# resulting in the inner function being passed to maxsize instead of an
513-
# integer or None.
514-
if maxsize is not None and not isinstance(maxsize, int):
513+
# integer or None. Negative maxsize is treated as 0.
514+
if isinstance(maxsize, int):
515+
if maxsize < 0:
516+
maxsize = 0
517+
elif maxsize is not None:
515518
raise TypeError('Expected maxsize to be an integer or None')
516519

517520
def decorating_function(user_function):
@@ -578,6 +581,7 @@ def wrapper(*args, **kwds):
578581
link[NEXT] = root
579582
hits += 1
580583
return result
584+
misses += 1
581585
result = user_function(*args, **kwds)
582586
with lock:
583587
if key in cache:
@@ -615,7 +619,6 @@ def wrapper(*args, **kwds):
615619
# Use the cache_len bound method instead of the len() function
616620
# which could potentially be wrapped in an lru_cache itself.
617621
full = (cache_len() >= maxsize)
618-
misses += 1
619622
return result
620623

621624
def cache_info():

Lib/test/test_functools.py

Lines changed: 28 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1233,6 +1233,33 @@ def f(x):
12331233
self.assertEqual(misses, 4)
12341234
self.assertEqual(currsize, 2)
12351235

1236+
def test_lru_bug_35780(self):
1237+
# C version of the lru_cache was not checking to see if
1238+
# the user function call has already modified the cache
1239+
# (this arises in recursive calls and in multi-threading).
1240+
# This cause the cache to have orphan links not referenced
1241+
# by the cache dictionary.
1242+
1243+
once = True # Modified by f(x) below
1244+
1245+
@self.module.lru_cache(maxsize=10)
1246+
def f(x):
1247+
nonlocal once
1248+
rv = f'.{x}.'
1249+
if x == 20 and once:
1250+
once = False
1251+
rv = f(x)
1252+
return rv
1253+
1254+
# Fill the cache
1255+
for x in range(15):
1256+
self.assertEqual(f(x), f'.{x}.')
1257+
self.assertEqual(f.cache_info().currsize, 10)
1258+
1259+
# Make a recursive call and make sure the cache remains full
1260+
self.assertEqual(f(20), '.20.')
1261+
self.assertEqual(f.cache_info().currsize, 10)
1262+
12361263
def test_lru_hash_only_once(self):
12371264
# To protect against weird reentrancy bugs and to improve
12381265
# efficiency when faced with slow __hash__ methods, the
@@ -1329,7 +1356,7 @@ def eq(n):
13291356
for i in (0, 1):
13301357
self.assertEqual([eq(n) for n in range(150)], list(range(150)))
13311358
self.assertEqual(eq.cache_info(),
1332-
self.module._CacheInfo(hits=0, misses=300, maxsize=-10, currsize=1))
1359+
self.module._CacheInfo(hits=0, misses=300, maxsize=0, currsize=0))
13331360

13341361
def test_lru_with_exceptions(self):
13351362
# Verify that user_function exceptions get passed through without
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
Fix lru_cache() errors arising in recursive, reentrant, or
2+
multi-threaded code. These errors could result in orphan links and in
3+
the cache being trapped in a state with fewer than the specified maximum
4+
number of links. Fix handling of negative maxsize which should have
5+
been treated as zero. Fix errors in toggling the "full" status flag.
6+
Fix misordering of links when errors are encountered. Sync-up the C
7+
code and pure Python code for the space saving path in functions with a
8+
single positional argument. In this common case, the space overhead of
9+
an lru cache entry is reduced by almost half. Fix counting of cache
10+
misses. In error cases, the miss count was out of sync with the actual
11+
number of times the underlying user function was called.

0 commit comments

Comments
 (0)