Skip to content

fix: Data leak in ThreadingIntegration between threads #4281

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 36 commits into from
Apr 15, 2025
Merged
Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
7d14008
Some test showing trx in trx is a bad idea
antonpirker Apr 11, 2025
9c35e83
test that shows we are leaking scope changes
antonpirker Apr 11, 2025
664f0e0
remove flaky tests, it does more harm then it helps
antonpirker Apr 11, 2025
0d9a221
better readability
antonpirker Apr 11, 2025
0100bdb
better readable
antonpirker Apr 11, 2025
bd44959
Propagate a copy of the scopes to threads
antonpirker Apr 11, 2025
0e92608
More tests
antonpirker Apr 11, 2025
fd69f64
better span names
antonpirker Apr 11, 2025
e4d94a5
what is happening?
antonpirker Apr 11, 2025
c7af4b9
back to forking
antonpirker Apr 11, 2025
678f515
trigger ci
antonpirker Apr 14, 2025
eef7ff6
trying without new tests
antonpirker Apr 14, 2025
518dc04
Revert all the changes
antonpirker Apr 14, 2025
24ea433
Trying somethign
antonpirker Apr 14, 2025
0d5b3fb
Reenable tests
antonpirker Apr 14, 2025
d3585a3
cleanup
antonpirker Apr 14, 2025
5bcba35
trying test forking
antonpirker Apr 14, 2025
4c2f7d9
Removed some stuff again
antonpirker Apr 14, 2025
890640f
commented wrong lines
antonpirker Apr 14, 2025
957d7bb
does it work in django 3.0?
antonpirker Apr 14, 2025
07d44c9
no channels in django 3.0
antonpirker Apr 14, 2025
2a2cde1
trying without channels
antonpirker Apr 14, 2025
8cd789b
No channels for newer Django versions
antonpirker Apr 14, 2025
6982479
Warning
antonpirker Apr 14, 2025
3b85ef4
Better error message
antonpirker Apr 15, 2025
e783af9
.
antonpirker Apr 15, 2025
160e430
checking for warning
antonpirker Apr 15, 2025
8cd1584
Merge branch 'master' into antonpirker/threading-tests
antonpirker Apr 15, 2025
9aad099
linting
antonpirker Apr 15, 2025
0da1c2e
explanation
antonpirker Apr 15, 2025
096579e
Revert tox to version in master
antonpirker Apr 15, 2025
33d9f3d
Merge branch 'master' into antonpirker/threading-tests
antonpirker Apr 15, 2025
815e930
Better checks and importing only once
antonpirker Apr 15, 2025
5d306ee
Merge branch 'antonpirker/threading-tests' of github.com:getsentry/se…
antonpirker Apr 15, 2025
8aa5edf
Better check
antonpirker Apr 15, 2025
ca8aa30
Merge branch 'master' into antonpirker/threading-tests
antonpirker Apr 15, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions sentry_sdk/integrations/threading.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,8 @@ 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()
isolation_scope = sentry_sdk.get_isolation_scope().fork()
current_scope = sentry_sdk.get_current_scope().fork()
else:
isolation_scope = None
current_scope = None
Expand Down
101 changes: 101 additions & 0 deletions tests/integrations/threading/test_threading.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import gc
from concurrent import futures
from textwrap import dedent
from threading import Thread

import pytest
Expand Down Expand Up @@ -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"\
"""
)
Loading