From 7ff22d6da41cd5ca80db95c18b409aea38e49fcd Mon Sep 17 00:00:00 2001 From: Sean Quah <8349537+squahtx@users.noreply.github.com> Date: Tue, 30 Nov 2021 16:28:02 +0000 Subject: Fix `LruCache` corruption bug with a `size_callback` that can return 0 (#11454) When all entries in an `LruCache` have a size of 0 according to the provided `size_callback`, and `drop_from_cache` is called on a cache node, the node would be unlinked from the LRU linked list but remain in the cache dictionary. An assertion would be later be tripped due to the inconsistency. Avoid unintentionally calling `__len__` and use a strict `is None` check instead when unwrapping the weak reference. --- tests/util/test_lrucache.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) (limited to 'tests/util') diff --git a/tests/util/test_lrucache.py b/tests/util/test_lrucache.py index 6578f3411e..291644eb7d 100644 --- a/tests/util/test_lrucache.py +++ b/tests/util/test_lrucache.py @@ -13,6 +13,7 @@ # limitations under the License. +from typing import List from unittest.mock import Mock from synapse.util.caches.lrucache import LruCache, setup_expire_lru_cache_entries @@ -261,6 +262,17 @@ class LruCacheSizedTestCase(unittest.HomeserverTestCase): self.assertEquals(cache["key4"], [4]) self.assertEquals(cache["key5"], [5, 6]) + def test_zero_size_drop_from_cache(self) -> None: + """Test that `drop_from_cache` works correctly with 0-sized entries.""" + cache: LruCache[str, List[int]] = LruCache(5, size_callback=lambda x: 0) + cache["key1"] = [] + + self.assertEqual(len(cache), 0) + cache.cache["key1"].drop_from_cache() + self.assertIsNone( + cache.pop("key1"), "Cache entry should have been evicted but wasn't" + ) + class TimeEvictionTestCase(unittest.HomeserverTestCase): """Test that time based eviction works correctly.""" -- cgit 1.5.1