Skip to content

SessionFlusher#add_session races with the flush swap #3038

Description

@SeanLF

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

Metadata

Metadata

Assignees

No one assigned

    Projects

    Status
    Waiting for: Product Owner

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions