Skip to content

fix(sessions): Lock pending aggregates in SessionFlusher#add_session - #3172

Merged
sl0thentr0py merged 1 commit into
masterfrom
fix/session-flusher-add-session-race
Nov 3, 2026
Merged

sl0thentr0py merged 1 commit into
masterfrom
fix/session-flusher-add-session-race

Conversation

@sentry-junior

@sentry-junior sentry-junior Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

Fixes #3038

SessionFlusher#add_session read and wrote @pending_aggregates in two lookups without holding @mutex. Only the flush swap in #pending_envelope took the lock, so the lock didn't stop anything. If the swap landed between the two lookups, the second one returned nil and raised NoMethodError: undefined method '[]' for nil from Hub#end_session, which runs in the Rack middleware's ensure. A swap at other points dropped the session silently.

add_session now holds @mutex around both lookups, which matches how TelemetryEventBuffer locks both sides. The new spec starts the flush swap on another thread while add_session is mid-update. It fails on master with the NoMethodError and passes with this change.

Proof: reproduction in getsentry/repro#65 (sentry-ruby/3038/repro.rb). It runs a deterministic interleaving plus a stress run with no hooks: one thread calls add_session while another runs the real swap.

Before (7.2.0, Ruby 4.0.9):

== 1. deterministic: swap lands between the two lookups in add_session
BUG: add_session raised NoMethodError: undefined method '[]' for nil
     at .../sentry-ruby-7.2.0/lib/sentry/session_flusher.rb:33:in 'Sentry::SessionFlusher#add_session'

== 2. stress: real add_session thread vs. real flush-swap thread (5.0s)
sessions added without error: 3144184
add_session crashes (NoMethodError): 7
sessions missing from flushed envelopes: 12

RESULT: FAIL - bug reproduced (2/2 scenarios)

After (this branch, Ruby 4.0.9):

== 1. deterministic: swap lands between the two lookups in add_session
OK: add_session did not raise and the session was flushed

== 2. stress: real add_session thread vs. real flush-swap thread (5.0s)
sessions added without error: 2798919
add_session crashes (NoMethodError): 0
sessions missing from flushed envelopes: 0

RESULT: PASS - SessionFlusher is safe against the flush swap

With RUBY_THREAD_TIMESLICE=1 STRESS_SECONDS=15, which makes MRI switch threads far more often, 7.2.0 had 286 crashes and 225 lost sessions out of about 8.6M. This branch had 0 and 0 out of about 8.4M. Ruby 3.2.11 shows the same before/after result.

via @dingsdax.

--

View Session [Sentry]

add_session read and wrote @pending_aggregates without holding @Mutex,
while the flush swap replaced the hash under it. A swap between the two
lookups made the second one return nil and raise NoMethodError from
Hub#end_session, and swaps at other points dropped sessions.

Fixes #3038

Co-Authored-By: Johannes Grin Dreadböck <114897+dingsdax@users.noreply.github.com>
@sl0thentr0py
sl0thentr0py merged commit 0c76f63 into master Nov 3, 2026
157 checks passed
@sl0thentr0py
sl0thentr0py deleted the fix/session-flusher-add-session-race branch November 3, 2026 11:42
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.

SessionFlusher#add_session races with the flush swap

2 participants