diff --git a/sentry-rails/lib/sentry/rails/active_job.rb b/sentry-rails/lib/sentry/rails/active_job.rb index 46a798895..042ef596f 100644 --- a/sentry-rails/lib/sentry/rails/active_job.rb +++ b/sentry-rails/lib/sentry/rails/active_job.rb @@ -117,13 +117,7 @@ def log_producer_span_error(e) end def record(job, trace_headers: nil, user: nil, &block) - # Always give this thread a fresh hub cloned from the main hub so - # the job's events are fully isolated. Save and restore whatever - # hub was on the thread before (e.g. the Rack request hub set by - # CaptureExceptions, or a stale hub left by a recycled thread-pool - # thread) so the outer context continues working correctly after - # the job finishes. - original_hub = Thread.current.thread_variable_get(Sentry::THREAD_LOCAL) + original_hub = Sentry.get_current_hub_internal Sentry.clone_hub_to_current_thread Sentry.with_scope do |scope| @@ -171,7 +165,7 @@ def record(job, trace_headers: nil, user: nil, &block) end end ensure - Thread.current.thread_variable_set(Sentry::THREAD_LOCAL, original_hub) + Sentry.set_current_hub_internal(original_hub) end def set_messaging_data(transaction, job) diff --git a/sentry-ruby/lib/sentry-ruby.rb b/sentry-ruby/lib/sentry-ruby.rb index 804b06bb9..83d075320 100644 --- a/sentry-ruby/lib/sentry-ruby.rb +++ b/sentry-ruby/lib/sentry-ruby.rb @@ -271,7 +271,8 @@ def init(&block) client = Client.new(config) scope = Scope.new(max_breadcrumbs: config.max_breadcrumbs) hub = Hub.new(client, scope) - Thread.current.thread_variable_set(THREAD_LOCAL, hub) + @hub_isolation_level = config.hub_isolation_level + set_current_hub_internal(hub) @main_hub = hub @background_worker = Sentry::BackgroundWorker.new(config) @session_flusher = config.session_tracking? ? Sentry::SessionFlusher.new(config, client) : nil @@ -309,7 +310,7 @@ def close MUTEX.synchronize do @main_hub = nil - Thread.current.thread_variable_set(THREAD_LOCAL, nil) + set_current_hub_internal(nil) end end @@ -362,7 +363,7 @@ def get_current_hub # ideally, we should do this proactively whenever a new thread is created # but it's impossible for the SDK to keep track every new thread # so we need to use this rather passive way to make sure the app doesn't crash - Thread.current.thread_variable_get(THREAD_LOCAL) || clone_hub_to_current_thread + get_current_hub_internal || clone_hub_to_current_thread end # Returns the current active client. @@ -380,12 +381,14 @@ def get_current_scope get_current_hub.current_scope end - # Clones the main thread's active hub and stores it to the current thread. + # Clones the main hub and stores it for the current execution context + # (the current thread, or the current fiber when +config.hub_isolation_level+ + # is +:fiber+). # # @return [void] def clone_hub_to_current_thread return unless initialized? - Thread.current.thread_variable_set(THREAD_LOCAL, get_main_hub.clone) + set_current_hub_internal(get_main_hub.clone) end # Takes a block and yields the current active scope. @@ -737,6 +740,35 @@ def utc_now def dependency_installed?(name) Object.const_defined?(name) end + + # Reads the hub stored for the current execution context. The active + # isolation level (cached from +config.hub_isolation_level+ at init) decides + # whether that context is the current thread or the current fiber. Reads the + # cached level rather than the configuration to avoid recursing back through + # hub resolution. + # + # @!visibility private + # @return [Hub, nil] + def get_current_hub_internal + if @hub_isolation_level == :fiber + ::Fiber[THREAD_LOCAL] + else + ::Thread.current.thread_variable_get(THREAD_LOCAL) + end + end + + # Stores +hub+ for the current execution context (thread or fiber). + # + # @!visibility private + # @param hub [Hub, nil] + # @return [Hub, nil] + def set_current_hub_internal(hub) + if @hub_isolation_level == :fiber + ::Fiber[THREAD_LOCAL] = hub + else + ::Thread.current.thread_variable_set(THREAD_LOCAL, hub) + end + end end end diff --git a/sentry-ruby/lib/sentry/configuration.rb b/sentry-ruby/lib/sentry/configuration.rb index 333bd9fb1..1f29d1854 100644 --- a/sentry-ruby/lib/sentry/configuration.rb +++ b/sentry-ruby/lib/sentry/configuration.rb @@ -389,6 +389,23 @@ class Configuration # @return [Boolean] attr_accessor :strict_trace_continuation + # Which execution primitive owns the SDK's current hub. + # + # [+:thread+ (default)] Store the hub in thread-local storage. Correct for + # thread-based servers (Puma, Unicorn) and background processors (Sidekiq, + # Resque). Every fiber on a thread shares one hub. + # [+:fiber+] Store the hub in Fiber Storage (Ruby 3.2+). Each fiber gets its + # own hub and child fibers inherit it, so concurrent requests on a + # fiber-based server (Falcon/async) are isolated instead of sharing and + # corrupting one another's scope. Requested on a Ruby without Fiber + # Storage (< 3.2), the SDK logs a warning and falls back to +:thread+. + # + # @return [Symbol] + attr_reader :hub_isolation_level + + # Isolation levels the SDK understands for hub storage. + ISOLATION_LEVELS = %i[thread fiber].freeze + # these are not config options # @!visibility private attr_reader :errors, :gem_specs @@ -541,6 +558,7 @@ def initialize self.capture_queue_time = true self.org_id = nil self.strict_trace_continuation = false + self.hub_isolation_level = :thread spotlight_env = ENV["SENTRY_SPOTLIGHT"] spotlight_bool = Sentry::Utils::EnvHelper.env_to_bool(spotlight_env, strict: true) @@ -610,6 +628,23 @@ def release=(value) @release = value end + def hub_isolation_level=(level) + level = level.to_sym if level.respond_to?(:to_sym) + + unless ISOLATION_LEVELS.include?(level) + raise ArgumentError, "hub_isolation_level must be one of #{ISOLATION_LEVELS.inspect}, got #{level.inspect}" + end + + # :fiber relies on Fiber Storage (Ruby 3.2+); downgrade so the hub is never + # asked to call Fiber[] on a Ruby that lacks it. + if level == :fiber && !fiber_storage_available? + log_warn("hub_isolation_level :fiber requires Ruby 3.2+ Fiber Storage; falling back to :thread on Ruby #{RUBY_VERSION}.") + level = :thread + end + + @hub_isolation_level = level + end + def breadcrumbs_logger=(logger) loggers = if logger.is_a?(Array) @@ -806,6 +841,10 @@ def run_after_close_callbacks private + def fiber_storage_available? + ::Fiber.respond_to?(:[]) && ::Fiber.respond_to?(:[]=) + end + def init_dsn(dsn_string) return if dsn_string.nil? || dsn_string.empty? diff --git a/sentry-ruby/lib/sentry/test_helper.rb b/sentry-ruby/lib/sentry/test_helper.rb index 4ede32933..92ecaf85c 100644 --- a/sentry-ruby/lib/sentry/test_helper.rb +++ b/sentry-ruby/lib/sentry/test_helper.rb @@ -52,10 +52,10 @@ def setup_sentry_test(&block) test_client = Sentry::Client.new(dummy_config.dup) main_hub.bind_client(test_client) - # Realign the current thread's hub with the main hub so direct + # Realign the current execution context's hub with the main hub so direct # `sentry_events` reads and any hub the Rack middleware clones from the # main hub all observe the same DummyTransport. - Thread.current.thread_variable_set(Sentry::THREAD_LOCAL, main_hub) + Sentry.set_current_hub_internal(main_hub) end # Clears all stored events and envelopes. @@ -165,7 +165,7 @@ def reset_sentry_globals! Sentry.instance_variable_set(:"@#{var}", nil) end - Thread.current.thread_variable_set(Sentry::THREAD_LOCAL, nil) + Sentry.set_current_hub_internal(nil) end end end diff --git a/sentry-ruby/spec/sentry/configuration_spec.rb b/sentry-ruby/spec/sentry/configuration_spec.rb index ffc09d5df..0b147417d 100644 --- a/sentry-ruby/spec/sentry/configuration_spec.rb +++ b/sentry-ruby/spec/sentry/configuration_spec.rb @@ -696,6 +696,42 @@ class SentryConfigurationSample < Sentry::Configuration end end + describe "#hub_isolation_level" do + it "defaults to :thread" do + expect(subject.hub_isolation_level).to eq(:thread) + end + + it "accepts :thread" do + subject.hub_isolation_level = :thread + expect(subject.hub_isolation_level).to eq(:thread) + end + + it "raises ArgumentError for an unknown level" do + expect { subject.hub_isolation_level = :process } + .to raise_error(ArgumentError, /hub_isolation_level must be one of/) + end + + context "when Fiber storage is available", when: { fiber_storage?: [] } do + it "accepts :fiber" do + subject.hub_isolation_level = :fiber + expect(subject.hub_isolation_level).to eq(:fiber) + end + + it "coerces string values" do + subject.hub_isolation_level = "fiber" + expect(subject.hub_isolation_level).to eq(:fiber) + end + end + + context "when Fiber storage is unavailable", when: { no_fiber_storage?: [] } do + it "falls back to :thread with a warning" do + expect(subject).to receive(:log_warn).with(/requires Ruby 3\.2\+ Fiber Storage/) + subject.hub_isolation_level = :fiber + expect(subject.hub_isolation_level).to eq(:thread) + end + end + end + describe "#validate" do it "logs a warning if StackProf is not installed" do allow(Sentry).to receive(:dependency_installed?).with(:StackProf).and_return(false) diff --git a/sentry-ruby/spec/sentry_spec.rb b/sentry-ruby/spec/sentry_spec.rb index 9917798e0..0b0403063 100644 --- a/sentry-ruby/spec/sentry_spec.rb +++ b/sentry-ruby/spec/sentry_spec.rb @@ -110,6 +110,66 @@ end end + describe "fiber isolation", when: { fiber_storage?: [] } do + before do + perform_basic_setup { |config| config.hub_isolation_level = :fiber } + end + + after do + described_class.set_current_hub_internal(nil) + described_class.instance_variable_set(:@hub_isolation_level, :thread) + end + + # Regression for cross-request contamination on fiber-based servers (Falcon, + # async), where many concurrent requests run as sibling fibers on one thread. + # With thread-local storage they share a single hub, so a scope opened by one + # request and held across a reactor yield is visible to (and clobbered by) the + # others. Fiber storage gives each request fiber its own hub. + it "keeps each sibling fiber's scope isolated across a yield" do + transport = described_class.get_main_hub.current_client.transport + transport.events.clear + + requests = 3.times.map do |i| + Fiber.new do + described_class.clone_hub_to_current_thread + described_class.configure_scope { |scope| scope.set_user(id: i) } + Fiber.yield # simulate yielding to the reactor mid-request + described_class.capture_message(i.to_s) + end + end + + requests.each(&:resume) # every request sets its user, then yields + requests.each(&:resume) # every request now captures its event + + attributed = transport.events.to_h { |e| [e.message.to_i, e.user[:id]] } + expect(attributed).to eq({ 0 => 0, 1 => 1, 2 => 2 }) + end + + it "lets a child fiber inherit the parent request's hub" do + described_class.clone_hub_to_current_thread + parent_hub = described_class.get_current_hub + + inherited = Fiber.new { described_class.get_current_hub }.resume + + expect(inherited).to eq(parent_hub) + end + + it "stores the hub in a fiber variable (instead of a thread variable)" do + described_class.set_tags(outside_fiber: true) + + fiber = Fiber.new do + described_class.clone_hub_to_current_thread + described_class.set_tags(inside_fiber: true) + described_class.get_current_scope.tags + end + + inside_tags = fiber.resume + + expect(inside_tags).to eq({ outside_fiber: true, inside_fiber: true }) + expect(described_class.get_current_scope.tags).to eq({ outside_fiber: true }) + end + end + shared_examples "capture_helper" do context "with sending_allowed? condition" do before do diff --git a/sentry-ruby/spec/spec_helper.rb b/sentry-ruby/spec/spec_helper.rb index c2cc7a056..d81f5cbe6 100644 --- a/sentry-ruby/spec/spec_helper.rb +++ b/sentry-ruby/spec/spec_helper.rb @@ -125,6 +125,14 @@ def self.ruby_version?(op, version) RUBY_VERSION.public_send(op, version) end + def self.fiber_storage? + Fiber.respond_to?(:[]) && Fiber.respond_to?(:[]=) + end + + def self.no_fiber_storage? + !fiber_storage? + end + def self.ruby_engine?(engine) RUBY_ENGINE == engine end