Skip to content

fix(profiling): Fix race condition while processing profiles #4553

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

Closed
wants to merge 1 commit into from

Conversation

szokeasaurusrex
Copy link
Member

If multiple threads happen to be processing profiles, we could encounter a race condition, that would lead to more profiles being processed than are available in the new_profiles deque. To avoid this, we can break upon encountering an IndexError.

This line appears to be the most likely cause of #4542 – in any case, handling an IndexError is not harmful

Fixes #4542


Thank you for contributing to sentry-python! Please add tests to validate your changes, and lint your code using tox -e linters.

Running the test suite on your PR might require maintainer approval.

If multiple threads happen to be processing profiles, we could encounter a race condition, that would lead to more profiles being processed than are available in the `new_profiles` deque. To avoid this, we can break upon encountering an `IndexError`.

This line appears to be the most likely cause of #4542 – in any case, handling an `IndexError` is not harmful

Fixes #4542
@szokeasaurusrex szokeasaurusrex requested a review from a team as a code owner July 7, 2025 15:48
@szokeasaurusrex szokeasaurusrex requested review from sentrivana and sl0thentr0py and removed request for a team July 7, 2025 15:48
@sl0thentr0py
Copy link
Member

sl0thentr0py commented Jul 7, 2025

@Zylphrex already fixed a similar issue here and was released in https://github.com/getsentry/sentry-python/releases/tag/2.31.0.

The report in #4542 is on 2.30.0 so I believe this is not needed, I would ask them to test on latest version again.

@szokeasaurusrex
Copy link
Member Author

True, nice catch @sl0thentr0py!

Copy link

codecov bot commented Jul 7, 2025

Codecov Report

Attention: Patch coverage is 60.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 80.66%. Comparing base (9e1cedd) to head (1566a93).

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
sentry_sdk/profiler/continuous_profiler.py 60.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4553      +/-   ##
==========================================
+ Coverage   80.63%   80.66%   +0.02%     
==========================================
  Files         156      156              
  Lines       16482    16486       +4     
  Branches     2802     2802              
==========================================
+ Hits        13291    13299       +8     
+ Misses       2308     2302       -6     
- Partials      883      885       +2     
Files with missing lines Coverage Δ
sentry_sdk/profiler/continuous_profiler.py 86.15% <60.00%> (-0.46%) ⬇️

... and 6 files with indirect coverage changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

IndexError: pop from an empty deque (continuous profiling?)
2 participants