fix: Data leak in ThreadingIntegration between threads (#4281)

It is possible to leak data from started threads into the main thread
via the scopes. (Because the same scope object from the main thread
could be changed in the started thread.)
This change always makes a fork (copy) of the scopes of the main thread
before it propagates those scopes into the started thread.

Author: antonpirker

sentry_sdk/integrations/threading.py

@@ -1,4 +1,5 @@
 import sys
+import warnings
 from functools import wraps
 from threading import Thread, current_thread
 
@@ -49,6 +50,15 @@ def setup_once():
         # type: () -> None
         old_start = Thread.start
 
+        try:
+            from django import VERSION as django_version  # noqa: N811
+            import channels  # type: ignore[import-not-found]
+
+            channels_version = channels.__version__
+        except ImportError:
+            django_version = None
+            channels_version = None
+
         @wraps(old_start)
         def sentry_start(self, *a, **kw):
             # type: (Thread, *Any, **Any) -> Any
@@ -57,8 +67,27 @@ def sentry_start(self, *a, **kw):
                 return old_start(self, *a, **kw)
 
             if integration.propagate_scope:
-                isolation_scope = sentry_sdk.get_isolation_scope()
-                current_scope = sentry_sdk.get_current_scope()
+                if (
+                    sys.version_info < (3, 9)
+                    and channels_version is not None
+                    and channels_version < "4.0.0"
+                    and django_version is not None
+                    and django_version >= (3, 0)
+                    and django_version < (4, 0)
+                ):
+                    warnings.warn(
+                        "There is a known issue with Django channels 2.x and 3.x when using Python 3.8 or older. "
+                        "(Async support is emulated using threads and some Sentry data may be leaked between those threads.) "
+                        "Please either upgrade to Django channels 4.0+, use Django's async features "
+                        "available in Django 3.1+ instead of Django channels, or upgrade to Python 3.9+.",
+                        stacklevel=2,
+                    )
+                    isolation_scope = sentry_sdk.get_isolation_scope()
+                    current_scope = sentry_sdk.get_current_scope()
+
+                else:
+                    isolation_scope = sentry_sdk.get_isolation_scope().fork()
+                    current_scope = sentry_sdk.get_current_scope().fork()
             else:
                 isolation_scope = None
                 current_scope = None

tests/integrations/django/asgi/test_asgi.py

@@ -38,9 +38,25 @@ async def test_basic(sentry_init, capture_events, application):
 
     events = capture_events()
 
-    comm = HttpCommunicator(application, "GET", "/view-exc?test=query")
-    response = await comm.get_response()
-    await comm.wait()
+    import channels  # type: ignore[import-not-found]
+
+    if (
+        sys.version_info < (3, 9)
+        and channels.__version__ < "4.0.0"
+        and django.VERSION >= (3, 0)
+        and django.VERSION < (4, 0)
+    ):
+        # We emit a UserWarning for channels 2.x and 3.x on Python 3.8 and older
+        # because the async support was not really good back then and there is a known issue.
+        # See the TreadingIntegration for details.
+        with pytest.warns(UserWarning):
+            comm = HttpCommunicator(application, "GET", "/view-exc?test=query")
+            response = await comm.get_response()
+            await comm.wait()
+    else:
+        comm = HttpCommunicator(application, "GET", "/view-exc?test=query")
+        response = await comm.get_response()
+        await comm.wait()
 
     assert response["status"] == 500
 

tests/integrations/threading/test_threading.py

@@ -1,5 +1,6 @@
 import gc
 from concurrent import futures
+from textwrap import dedent
 from threading import Thread
 
 import pytest
@@ -172,3 +173,103 @@ def target():
     assert Thread.run.__qualname__ == original_run.__qualname__
     assert t.run.__name__ == "run"
     assert t.run.__qualname__ == original_run.__qualname__
+
+
+@pytest.mark.parametrize(
+    "propagate_scope",
+    (True, False),
+    ids=["propagate_scope=True", "propagate_scope=False"],
+)
+def test_scope_data_not_leaked_in_threads(sentry_init, propagate_scope):
+    sentry_init(
+        integrations=[ThreadingIntegration(propagate_scope=propagate_scope)],
+    )
+
+    sentry_sdk.set_tag("initial_tag", "initial_value")
+    initial_iso_scope = sentry_sdk.get_isolation_scope()
+
+    def do_some_work():
+        # check if we have the initial scope data propagated into the thread
+        if propagate_scope:
+            assert sentry_sdk.get_isolation_scope()._tags == {
+                "initial_tag": "initial_value"
+            }
+        else:
+            assert sentry_sdk.get_isolation_scope()._tags == {}
+
+        # change data in isolation scope in thread
+        sentry_sdk.set_tag("thread_tag", "thread_value")
+
+    t = Thread(target=do_some_work)
+    t.start()
+    t.join()
+
+    # check if the initial scope data is not modified by the started thread
+    assert initial_iso_scope._tags == {
+        "initial_tag": "initial_value"
+    }, "The isolation scope in the main thread should not be modified by the started thread."
+
+
+@pytest.mark.parametrize(
+    "propagate_scope",
+    (True, False),
+    ids=["propagate_scope=True", "propagate_scope=False"],
+)
+def test_spans_from_multiple_threads(
+    sentry_init, capture_events, render_span_tree, propagate_scope
+):
+    sentry_init(
+        traces_sample_rate=1.0,
+        integrations=[ThreadingIntegration(propagate_scope=propagate_scope)],
+    )
+    events = capture_events()
+
+    def do_some_work(number):
+        with sentry_sdk.start_span(
+            op=f"inner-run-{number}", name=f"Thread: child-{number}"
+        ):
+            pass
+
+    threads = []
+
+    with sentry_sdk.start_transaction(op="outer-trx"):
+        for number in range(5):
+            with sentry_sdk.start_span(
+                op=f"outer-submit-{number}", name="Thread: main"
+            ):
+                t = Thread(target=do_some_work, args=(number,))
+                t.start()
+                threads.append(t)
+
+        for t in threads:
+            t.join()
+
+    (event,) = events
+    if propagate_scope:
+        assert render_span_tree(event) == dedent(
+            """\
+            - op="outer-trx": description=null
+              - op="outer-submit-0": description="Thread: main"
+                - op="inner-run-0": description="Thread: child-0"
+              - op="outer-submit-1": description="Thread: main"
+                - op="inner-run-1": description="Thread: child-1"
+              - op="outer-submit-2": description="Thread: main"
+                - op="inner-run-2": description="Thread: child-2"
+              - op="outer-submit-3": description="Thread: main"
+                - op="inner-run-3": description="Thread: child-3"
+              - op="outer-submit-4": description="Thread: main"
+                - op="inner-run-4": description="Thread: child-4"\
+"""
+        )
+
+    elif not propagate_scope:
+        assert render_span_tree(event) == dedent(
+            """\
+            - op="outer-trx": description=null
+              - op="outer-submit-0": description="Thread: main"
+              - op="outer-submit-1": description="Thread: main"
+              - op="outer-submit-2": description="Thread: main"
+              - op="outer-submit-3": description="Thread: main"
+              - op="outer-submit-4": description="Thread: main"\
+"""
+        )