From 8f9461e1a0bc497e6333b4d955561a904beb9dae Mon Sep 17 00:00:00 2001 From: Colton Allen Date: Fri, 6 Dec 2024 02:11:03 -0600 Subject: [PATCH] Deepcopy and ensure get_all function always terminates (#3861) @aliu39 discovered that under certain circumstances a process can get stuck in an infinite loop. Andrew fixed this by using `deepcopy` which prevents the infinite loop and fixes a bug where the LRU returns incorrect results. Additionally, I've added a terminating loop in case there are any future bugs we've missed. Closes: https://github.com/getsentry/sentry-python/issues/3862 Out of precaution, we disabled flagpole evaluation tracking Sentry while we wait for this to be merged. --- sentry_sdk/_lru_cache.py | 14 +++++++++++--- tests/test_lru_cache.py | 18 ++++++++++++++++++ 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/sentry_sdk/_lru_cache.py b/sentry_sdk/_lru_cache.py index ec557b1093..825c773529 100644 --- a/sentry_sdk/_lru_cache.py +++ b/sentry_sdk/_lru_cache.py @@ -62,7 +62,7 @@ """ -from copy import copy +from copy import copy, deepcopy SENTINEL = object() @@ -95,7 +95,7 @@ def __copy__(self): cache = LRUCache(self.max_size) cache.full = self.full cache.cache = copy(self.cache) - cache.root = copy(self.root) + cache.root = deepcopy(self.root) return cache def set(self, key, value): @@ -167,7 +167,15 @@ def get(self, key, default=None): def get_all(self): nodes = [] node = self.root[NEXT] - while node is not self.root: + + # To ensure the loop always terminates we iterate to the maximum + # size of the LRU cache. + for _ in range(self.max_size): + # The cache may not be full. We exit early if we've wrapped + # around to the head. + if node is self.root: + break nodes.append((node[KEY], node[VALUE])) node = node[NEXT] + return nodes diff --git a/tests/test_lru_cache.py b/tests/test_lru_cache.py index 3e9c0ac964..cab9bbc7eb 100644 --- a/tests/test_lru_cache.py +++ b/tests/test_lru_cache.py @@ -1,4 +1,5 @@ import pytest +from copy import copy from sentry_sdk._lru_cache import LRUCache @@ -58,3 +59,20 @@ def test_cache_get_all(): assert cache.get_all() == [(1, 1), (2, 2), (3, 3)] cache.get(1) assert cache.get_all() == [(2, 2), (3, 3), (1, 1)] + + +def test_cache_copy(): + cache = LRUCache(3) + cache.set(0, 0) + cache.set(1, 1) + + copied = copy(cache) + cache.set(2, 2) + cache.set(3, 3) + assert copied.get_all() == [(0, 0), (1, 1)] + assert cache.get_all() == [(1, 1), (2, 2), (3, 3)] + + copied = copy(cache) + cache.get(1) + assert copied.get_all() == [(1, 1), (2, 2), (3, 3)] + assert cache.get_all() == [(2, 2), (3, 3), (1, 1)]