add_session (session_flusher.rb:26-33) writes @pending_aggregates with no lock, while pending_envelope (:44-49) swaps in a fresh Hash under @mutex. Only the reader takes the mutex, so it excludes nothing.
add_session does two lookups:
@pending_aggregates[session.aggregation_key] ||= init_aggregates(session.aggregation_key)
@pending_aggregates[session.aggregation_key][session.status] += 1
If the swap lands between them, the second returns nil. Reproduced as NoMethodError: undefined method '[]' for nil, raised inside Hub#end_session, which runs in the Rack middleware's ensure.
Rare in production, since it needs the 60s tick to land in a narrow window. The spec below drives the swap from init_aggregates to make the interleaving deterministic. TelemetryEventBuffer locks both sides, which suggests this is an oversight rather than intent.
Failing spec
Drop in sentry-ruby/spec/ and run bundle exec rspec spec/session_flusher_race_spec.rb. Fails on master with NoMethodError: undefined method '[]' for nil, deterministically.
# Run from sentry-ruby/: bundle exec rspec path/to/session_flusher_race_spec.rb
require "spec_helper"
RSpec.describe "SessionFlusher#add_session vs the flush swap" do
before { perform_basic_setup { |config| config.release = "1.0" } }
let(:flusher) do
Sentry::SessionFlusher.new(Sentry.configuration, Sentry.get_current_client).tap do |f|
def f.ensure_thread = true # skip the real flush thread
end
end
def exited_session
Sentry::Session.new.tap { |s| s.instance_variable_set(:@status, :exited) }
end
# add_session reads @pending_aggregates twice with no lock:
#
# @pending_aggregates[key] ||= init_aggregates(key)
# @pending_aggregates[key][status] += 1
#
# In production the flusher thread's 60s swap lands between them. Here the
# swap is driven from init_aggregates to make that interleaving deterministic
# rather than relying on a 1-in-20k stress race.
it "does not raise when the pending hash is swapped mid-update" do
allow(flusher).to receive(:init_aggregates).and_wrap_original do |original, *args|
original.call(*args).tap { flusher.send(:pending_envelope) }
end
expect { flusher.add_session(exited_session) }.not_to raise_error
end
end
add_session(session_flusher.rb:26-33) writes@pending_aggregateswith no lock, whilepending_envelope(:44-49) swaps in a fresh Hash under@mutex. Only the reader takes the mutex, so it excludes nothing.add_sessiondoes two lookups:If the swap lands between them, the second returns nil. Reproduced as
NoMethodError: undefined method '[]' for nil, raised insideHub#end_session, which runs in the Rack middleware'sensure.Rare in production, since it needs the 60s tick to land in a narrow window. The spec below drives the swap from
init_aggregatesto make the interleaving deterministic.TelemetryEventBufferlocks both sides, which suggests this is an oversight rather than intent.Failing spec
Drop in
sentry-ruby/spec/and runbundle exec rspec spec/session_flusher_race_spec.rb. Fails on master withNoMethodError: undefined method '[]' for nil, deterministically.