From 04d2596a9acd22cd96090b339951b8f52c614870 Mon Sep 17 00:00:00 2001
From: Colton Allen <colton.allen@sentry.io>
Date: Mon, 10 Feb 2025 14:16:24 -0600
Subject: [PATCH 1/3] Fix concurrent access bug

---
 sentry_sdk/feature_flags.py | 33 +++++++++++++++++++++++++++++----
 tests/test_feature_flags.py | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 63 insertions(+), 4 deletions(-)

diff --git a/sentry_sdk/feature_flags.py b/sentry_sdk/feature_flags.py
index 1187c2fa12..30a9f618df 100644
--- a/sentry_sdk/feature_flags.py
+++ b/sentry_sdk/feature_flags.py
@@ -1,5 +1,7 @@
+import copy
 import sentry_sdk
 from sentry_sdk._lru_cache import LRUCache
+from threading import Lock
 
 from typing import TYPE_CHECKING
 
@@ -16,20 +18,43 @@ class FlagBuffer:
 
     def __init__(self, capacity):
         # type: (int) -> None
-        self.buffer = LRUCache(capacity)
         self.capacity = capacity
+        self.lock = Lock()
+
+        # Buffer is private. The name is mangled to discourage use. If you use this attribute
+        # directly you're on your own!
+        self.__buffer = LRUCache(capacity)
 
     def clear(self):
         # type: () -> None
-        self.buffer = LRUCache(self.capacity)
+        self.__buffer = LRUCache(self.capacity)
+
+    def __deepcopy__(self, memo):
+        with self.lock:
+            buffer = FlagBuffer(self.capacity)
+            buffer.__buffer = copy.deepcopy(self.__buffer, memo)
+            return buffer
 
     def get(self):
         # type: () -> list[FlagData]
-        return [{"flag": key, "result": value} for key, value in self.buffer.get_all()]
+        with self.lock:
+            return [
+                {"flag": key, "result": value} for key, value in self.__buffer.get_all()
+            ]
 
     def set(self, flag, result):
         # type: (str, bool) -> None
-        self.buffer.set(flag, result)
+        if isinstance(result, FlagBuffer):
+            # If someone were to `self` in `self` this would create a circular dependency on
+            # the lock. This is of course a deadlock. However, this is far outside the expected
+            # usage of this class. We guard against it here for completeness and to document this
+            # expected failure mode.
+            raise ValueError(
+                "FlagBuffer instances can not be inserted into the dictionary."
+            )
+
+        with self.lock:
+            self.__buffer.set(flag, result)
 
 
 def add_feature_flag(flag, result):
diff --git a/tests/test_feature_flags.py b/tests/test_feature_flags.py
index 14d74cb04b..4469b5c2ca 100644
--- a/tests/test_feature_flags.py
+++ b/tests/test_feature_flags.py
@@ -1,5 +1,7 @@
 import concurrent.futures as cf
 import sys
+import copy
+import threading
 
 import pytest
 
@@ -167,3 +169,35 @@ def test_flag_tracking():
         {"flag": "e", "result": False},
         {"flag": "f", "result": False},
     ]
+
+
+def test_flag_buffer_concurrent_access():
+    buffer = FlagBuffer(capacity=100)
+    error_occurred = False
+
+    def writer():
+        for i in range(1_000_000):
+            buffer.set(f"key_{i}", True)
+
+    def reader():
+        nonlocal error_occurred
+
+        try:
+            for _ in range(1000):
+                copy.deepcopy(buffer)
+        except RuntimeError:
+            error_occurred = True
+
+    writer_thread = threading.Thread(target=writer)
+    reader_thread = threading.Thread(target=reader)
+
+    writer_thread.start()
+    reader_thread.start()
+
+    writer_thread.join(timeout=5)
+    reader_thread.join(timeout=5)
+
+    # This should always be false. If this ever fails we know we have concurrent access to a
+    # shared resource. When deepcopying we should have exclusive access to the underlying
+    # memory.
+    assert error_occurred is False

From 5ab429531dcea242f6bfe8299bfd2b5a13bbf0f6 Mon Sep 17 00:00:00 2001
From: Colton Allen <colton.allen@sentry.io>
Date: Mon, 10 Feb 2025 14:18:32 -0600
Subject: [PATCH 2/3] Word

---
 sentry_sdk/feature_flags.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sentry_sdk/feature_flags.py b/sentry_sdk/feature_flags.py
index 30a9f618df..348339a8a3 100644
--- a/sentry_sdk/feature_flags.py
+++ b/sentry_sdk/feature_flags.py
@@ -45,8 +45,8 @@ def get(self):
     def set(self, flag, result):
         # type: (str, bool) -> None
         if isinstance(result, FlagBuffer):
-            # If someone were to `self` in `self` this would create a circular dependency on
-            # the lock. This is of course a deadlock. However, this is far outside the expected
+            # If someone were to insert `self` into `self` this would create a circular dependency
+            # on the lock. This is of course a deadlock. However, this is far outside the expected
             # usage of this class. We guard against it here for completeness and to document this
             # expected failure mode.
             raise ValueError(

From dfd6ceffc0e2a9b02e95258b90e5428ab78adad0 Mon Sep 17 00:00:00 2001
From: Colton Allen <colton.allen@sentry.io>
Date: Mon, 10 Feb 2025 15:06:44 -0600
Subject: [PATCH 3/3] Add types

---
 sentry_sdk/feature_flags.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/sentry_sdk/feature_flags.py b/sentry_sdk/feature_flags.py
index 348339a8a3..a0b1338356 100644
--- a/sentry_sdk/feature_flags.py
+++ b/sentry_sdk/feature_flags.py
@@ -3,7 +3,7 @@
 from sentry_sdk._lru_cache import LRUCache
 from threading import Lock
 
-from typing import TYPE_CHECKING
+from typing import TYPE_CHECKING, Any
 
 if TYPE_CHECKING:
     from typing import TypedDict
@@ -30,6 +30,7 @@ def clear(self):
         self.__buffer = LRUCache(self.capacity)
 
     def __deepcopy__(self, memo):
+        # type: (dict[int, Any]) -> FlagBuffer
         with self.lock:
             buffer = FlagBuffer(self.capacity)
             buffer.__buffer = copy.deepcopy(self.__buffer, memo)