Skip to content

Commit 8f9461e

Browse files
authored
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: #3862 Out of precaution, we disabled flagpole evaluation tracking Sentry while we wait for this to be merged.
1 parent fd56608 commit 8f9461e

File tree

2 files changed

+29
-3
lines changed

2 files changed

+29
-3
lines changed

sentry_sdk/_lru_cache.py

+11-3
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@
6262
6363
"""
6464

65-
from copy import copy
65+
from copy import copy, deepcopy
6666

6767
SENTINEL = object()
6868

@@ -95,7 +95,7 @@ def __copy__(self):
9595
cache = LRUCache(self.max_size)
9696
cache.full = self.full
9797
cache.cache = copy(self.cache)
98-
cache.root = copy(self.root)
98+
cache.root = deepcopy(self.root)
9999
return cache
100100

101101
def set(self, key, value):
@@ -167,7 +167,15 @@ def get(self, key, default=None):
167167
def get_all(self):
168168
nodes = []
169169
node = self.root[NEXT]
170-
while node is not self.root:
170+
171+
# To ensure the loop always terminates we iterate to the maximum
172+
# size of the LRU cache.
173+
for _ in range(self.max_size):
174+
# The cache may not be full. We exit early if we've wrapped
175+
# around to the head.
176+
if node is self.root:
177+
break
171178
nodes.append((node[KEY], node[VALUE]))
172179
node = node[NEXT]
180+
173181
return nodes

tests/test_lru_cache.py

+18
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import pytest
2+
from copy import copy
23

34
from sentry_sdk._lru_cache import LRUCache
45

@@ -58,3 +59,20 @@ def test_cache_get_all():
5859
assert cache.get_all() == [(1, 1), (2, 2), (3, 3)]
5960
cache.get(1)
6061
assert cache.get_all() == [(2, 2), (3, 3), (1, 1)]
62+
63+
64+
def test_cache_copy():
65+
cache = LRUCache(3)
66+
cache.set(0, 0)
67+
cache.set(1, 1)
68+
69+
copied = copy(cache)
70+
cache.set(2, 2)
71+
cache.set(3, 3)
72+
assert copied.get_all() == [(0, 0), (1, 1)]
73+
assert cache.get_all() == [(1, 1), (2, 2), (3, 3)]
74+
75+
copied = copy(cache)
76+
cache.get(1)
77+
assert copied.get_all() == [(1, 1), (2, 2), (3, 3)]
78+
assert cache.get_all() == [(2, 2), (3, 3), (1, 1)]

0 commit comments

Comments
 (0)