diff --git a/sentry-rails/lib/sentry/rails/active_job.rb b/sentry-rails/lib/sentry/rails/active_job.rb index 46a798895..875cc824c 100644 --- a/sentry-rails/lib/sentry/rails/active_job.rb +++ b/sentry-rails/lib/sentry/rails/active_job.rb @@ -123,7 +123,7 @@ def record(job, trace_headers: nil, user: nil, &block) # 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::HubStorage.get Sentry.clone_hub_to_current_thread Sentry.with_scope do |scope| @@ -171,7 +171,7 @@ def record(job, trace_headers: nil, user: nil, &block) end end ensure - Thread.current.thread_variable_set(Sentry::THREAD_LOCAL, original_hub) + Sentry::HubStorage.set(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..a7580ccca 100644 --- a/sentry-ruby/lib/sentry-ruby.rb +++ b/sentry-ruby/lib/sentry-ruby.rb @@ -21,6 +21,7 @@ require "sentry/span" require "sentry/transaction" require "sentry/hub" +require "sentry/hub_storage" require "sentry/background_worker" require "sentry/threaded_periodic_worker" require "sentry/session_flusher" @@ -271,7 +272,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) + HubStorage.isolation_level = config.isolation_level + HubStorage.set(hub) @main_hub = hub @background_worker = Sentry::BackgroundWorker.new(config) @session_flusher = config.session_tracking? ? Sentry::SessionFlusher.new(config, client) : nil @@ -309,7 +311,7 @@ def close MUTEX.synchronize do @main_hub = nil - Thread.current.thread_variable_set(THREAD_LOCAL, nil) + HubStorage.clear end end @@ -362,7 +364,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 + HubStorage.get || clone_hub_to_current_thread end # Returns the current active client. @@ -380,12 +382,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.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) + HubStorage.set(get_main_hub.clone) end # Takes a block and yields the current active scope. diff --git a/sentry-ruby/lib/sentry/configuration.rb b/sentry-ruby/lib/sentry/configuration.rb index 333bd9fb1..bc81ddf39 100644 --- a/sentry-ruby/lib/sentry/configuration.rb +++ b/sentry-ruby/lib/sentry/configuration.rb @@ -389,6 +389,20 @@ 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 :isolation_level + # these are not config options # @!visibility private attr_reader :errors, :gem_specs @@ -541,6 +555,10 @@ def initialize self.capture_queue_time = true self.org_id = nil self.strict_trace_continuation = false + # Assign the ivar directly rather than through the setter: the default must + # not sync HubStorage (a throwaway Configuration.new would otherwise clobber + # the active isolation level). init and the setter handle syncing. + @isolation_level = :thread spotlight_env = ENV["SENTRY_SPOTLIGHT"] spotlight_bool = Sentry::Utils::EnvHelper.env_to_bool(spotlight_env, strict: true) @@ -610,6 +628,22 @@ def release=(value) @release = value end + def isolation_level=(level) + level = level.to_sym if level.respond_to?(:to_sym) + applied = Sentry::HubStorage.normalize_isolation_level(level) + + if level == :fiber && applied != :fiber + log_warn("isolation_level :fiber requires Ruby 3.2+ Fiber Storage; falling back to :thread on Ruby #{RUBY_VERSION}.") + end + + @isolation_level = applied + + # Keep the active hub storage in sync when the level is changed after the + # SDK is initialized. During `Sentry.init` the config block runs before the + # SDK is initialized, so init applies the level to HubStorage itself. + Sentry::HubStorage.isolation_level = applied if Sentry.initialized? + end + def breadcrumbs_logger=(logger) loggers = if logger.is_a?(Array) diff --git a/sentry-ruby/lib/sentry/hub_storage.rb b/sentry-ruby/lib/sentry/hub_storage.rb new file mode 100644 index 000000000..9c72759fb --- /dev/null +++ b/sentry-ruby/lib/sentry/hub_storage.rb @@ -0,0 +1,92 @@ +# frozen_string_literal: true + +module Sentry + # Stores the SDK's current {Hub} in an execution-context-local slot. + # + # The isolation level decides which execution primitive owns the hub: + # + # [+:thread+ (default)] The hub lives in thread-local storage + # (+Thread#thread_variable_get+/+thread_variable_set+). This is the SDK's + # historical behaviour and is correct for thread-based servers (Puma, + # Unicorn) and background processors (Sidekiq, Resque). Every fiber running + # on a thread shares one hub. + # + # [+:fiber+] The hub lives in Fiber Storage (+Fiber#[]+, Ruby 3.2+). Each + # fiber gets its own hub, and a newly created fiber inherits a copy of its + # parent's storage, so context still propagates into child fibers (for + # example graphql-ruby resolvers or +Async+ tasks) instead of being lost. + # This is the correct level for fiber-based servers such as Falcon, where + # many concurrent requests run as sibling fibers on a single thread and must + # not share a hub. + # + # Both levels use the same storage key (+Sentry::THREAD_LOCAL+, value + # +:sentry_hub+), kept stable for backwards compatibility because integrations + # and user code reference it directly. + # + # @api private + module HubStorage + # Isolation levels the SDK understands. + LEVELS = %i[thread fiber].freeze + + class << self + # @return [Symbol] the currently active isolation level. + attr_reader :isolation_level + + # @param level [Symbol] +:thread+ or +:fiber+. + # @raise [ArgumentError] if +level+ is not a known isolation level. + # @return [Symbol] the level that was applied. + def isolation_level=(level) + @isolation_level = normalize_isolation_level(level) + end + + # Validates a requested isolation level and downgrades +:fiber+ to + # +:thread+ when Fiber Storage is unavailable (Ruby < 3.2), so the storage + # boundary can never be asked to call +Fiber[]+ on a Ruby that lacks it. + # + # @param level [Symbol] + # @raise [ArgumentError] if +level+ is not a known isolation level. + # @return [Symbol] the level that is safe to apply. + def normalize_isolation_level(level) + unless LEVELS.include?(level) + raise ArgumentError, "isolation_level must be one of #{LEVELS.inspect}, got #{level.inspect}" + end + + level == :fiber && !fiber_storage_available? ? :thread : level + end + + # @return [Hub, nil] the hub stored for the current execution context. + def get + if @isolation_level == :fiber + ::Fiber[THREAD_LOCAL] + else + ::Thread.current.thread_variable_get(THREAD_LOCAL) + end + end + + # @param hub [Hub, nil] + # @return [Hub, nil] + def set(hub) + if @isolation_level == :fiber + ::Fiber[THREAD_LOCAL] = hub + else + ::Thread.current.thread_variable_set(THREAD_LOCAL, hub) + end + end + + # Clears the hub for the current execution context. + # @return [void] + def clear + set(nil) + end + + # Whether the running Ruby exposes Fiber Storage (+Fiber#[]+, Ruby 3.2+), + # the primitive the +:fiber+ isolation level is built on. + # @return [Boolean] + def fiber_storage_available? + ::Fiber.respond_to?(:[]) && ::Fiber.respond_to?(:[]=) + end + end + + self.isolation_level = :thread + end +end diff --git a/sentry-ruby/lib/sentry/test_helper.rb b/sentry-ruby/lib/sentry/test_helper.rb index 4ede32933..580b1178b 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::HubStorage.set(main_hub) end # Clears all stored events and envelopes. @@ -165,7 +165,11 @@ def reset_sentry_globals! Sentry.instance_variable_set(:"@#{var}", nil) end - Thread.current.thread_variable_set(Sentry::THREAD_LOCAL, nil) + # Clear the hub using the level that is still in effect, then restore + # the default so a `:fiber` test cannot leak its isolation level into + # subsequent tests that expect the default. + Sentry::HubStorage.clear + Sentry::HubStorage.isolation_level = :thread end end end diff --git a/sentry-ruby/spec/sentry/configuration_spec.rb b/sentry-ruby/spec/sentry/configuration_spec.rb index ffc09d5df..d38e70eea 100644 --- a/sentry-ruby/spec/sentry/configuration_spec.rb +++ b/sentry-ruby/spec/sentry/configuration_spec.rb @@ -696,6 +696,51 @@ class SentryConfigurationSample < Sentry::Configuration end end + describe "#isolation_level" do + it "defaults to :thread" do + expect(subject.isolation_level).to eq(:thread) + end + + it "accepts :thread and :fiber" do + # stub availability so the :fiber assertion is deterministic on Rubies < 3.2 + allow(Sentry::HubStorage).to receive(:fiber_storage_available?).and_return(true) + + subject.isolation_level = :fiber + expect(subject.isolation_level).to eq(:fiber) + + subject.isolation_level = :thread + expect(subject.isolation_level).to eq(:thread) + end + + it "coerces string values" do + allow(Sentry::HubStorage).to receive(:fiber_storage_available?).and_return(true) + subject.isolation_level = "fiber" + expect(subject.isolation_level).to eq(:fiber) + end + + it "raises ArgumentError for an unknown level" do + expect { subject.isolation_level = :process } + .to raise_error(ArgumentError, /isolation_level must be one of/) + end + + context "when Fiber storage is available" do + it "keeps :fiber" do + allow(Sentry::HubStorage).to receive(:fiber_storage_available?).and_return(true) + subject.isolation_level = :fiber + expect(subject.isolation_level).to eq(:fiber) + end + end + + context "when Fiber storage is unavailable" do + it "falls back to :thread with a warning" do + allow(Sentry::HubStorage).to receive(:fiber_storage_available?).and_return(false) + expect(subject).to receive(:log_warn).with(/requires Ruby 3\.2\+ Fiber Storage/) + subject.isolation_level = :fiber + expect(subject.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/hub_storage_spec.rb b/sentry-ruby/spec/sentry/hub_storage_spec.rb new file mode 100644 index 000000000..5585acfd0 --- /dev/null +++ b/sentry-ruby/spec/sentry/hub_storage_spec.rb @@ -0,0 +1,103 @@ +# frozen_string_literal: true + +RSpec.describe Sentry::HubStorage do + around do |example| + original = described_class.isolation_level + example.run + ensure + described_class.isolation_level = original + described_class.clear + end + + describe ".isolation_level=" do + it "defaults to :thread" do + # the module sets :thread on load + described_class.isolation_level = :thread + expect(described_class.isolation_level).to eq(:thread) + end + + it "accepts :fiber when Fiber storage is available" do + allow(described_class).to receive(:fiber_storage_available?).and_return(true) + described_class.isolation_level = :fiber + expect(described_class.isolation_level).to eq(:fiber) + end + + it "downgrades :fiber to :thread when Fiber storage is unavailable" do + allow(described_class).to receive(:fiber_storage_available?).and_return(false) + described_class.isolation_level = :fiber + expect(described_class.isolation_level).to eq(:thread) + end + + it "raises ArgumentError for an unknown level" do + expect { described_class.isolation_level = :process } + .to raise_error(ArgumentError, /isolation_level must be one of/) + end + end + + describe ".fiber_storage_available?", when: { fiber_storage?: [] } do + it "is true on Ruby 3.2+" do + expect(described_class.fiber_storage_available?).to be(true) + end + end + + describe ".get / .set / .clear" do + let(:hub) { instance_double(Sentry::Hub) } + + context "with :thread isolation" do + before { described_class.isolation_level = :thread } + + it "stores and reads the hub from thread-local storage" do + described_class.set(hub) + expect(described_class.get).to eq(hub) + expect(Thread.current.thread_variable_get(Sentry::THREAD_LOCAL)).to eq(hub) + end + + it "clears the hub" do + described_class.set(hub) + described_class.clear + expect(described_class.get).to be_nil + end + + it "isolates the hub per thread" do + described_class.set(hub) + other = Thread.new { described_class.get }.value + expect(other).to be_nil + end + end + + context "with :fiber isolation", when: { fiber_storage?: [] } do + before { described_class.isolation_level = :fiber } + + it "stores and reads the hub from fiber storage" do + described_class.set(hub) + expect(described_class.get).to eq(hub) + expect(Fiber[Sentry::THREAD_LOCAL]).to eq(hub) + end + + it "clears the hub" do + described_class.set(hub) + described_class.clear + expect(described_class.get).to be_nil + end + + it "isolates the hub between sibling fibers" do + described_class.set(hub) + + sibling = Fiber.new do + described_class.set(:sibling_hub) + described_class.get + end + + expect(sibling.resume).to eq(:sibling_hub) + # the sibling's write must not leak back into the current fiber + expect(described_class.get).to eq(hub) + end + + it "lets a child fiber inherit the parent's hub (the #1374 case)" do + described_class.set(hub) + child = Fiber.new { described_class.get } + expect(child.resume).to eq(hub) + end + end + end +end diff --git a/sentry-ruby/spec/sentry_spec.rb b/sentry-ruby/spec/sentry_spec.rb index 9917798e0..c9113e9ca 100644 --- a/sentry-ruby/spec/sentry_spec.rb +++ b/sentry-ruby/spec/sentry_spec.rb @@ -110,6 +110,64 @@ end end + describe "changing isolation_level after init", when: { fiber_storage?: [] } do + before { perform_basic_setup } + + after { Sentry::HubStorage.isolation_level = :thread } + + it "applies the change to the active hub storage" do + expect(Sentry::HubStorage.isolation_level).to eq(:thread) + + Sentry.configuration.isolation_level = :fiber + + expect(Sentry::HubStorage.isolation_level).to eq(:fiber) + end + end + + describe "fiber isolation", when: { fiber_storage?: [] } do + before do + perform_basic_setup { |config| config.isolation_level = :fiber } + end + + after do + Sentry::HubStorage.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("request #{i}") + 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[/request (\d+)/, 1].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 + 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..fddbc6ae1 100644 --- a/sentry-ruby/spec/spec_helper.rb +++ b/sentry-ruby/spec/spec_helper.rb @@ -125,6 +125,10 @@ 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.ruby_engine?(engine) RUBY_ENGINE == engine end