Skip to content

Commit 7ab412f

Browse files
SeanLFclaudesl0thentr0py
authored
feat: add config.hub_isolation_level for fiber-safe hub storage (Falcon/async) (#3018)
* feat: add config.isolation_level for fiber-safe hub storage The current hub is stored in thread-local storage. On a fiber-based server (Falcon/async) many concurrent requests run as sibling fibers on one thread, so they share a single hub. When a request holds a scope across a reactor yield (streaming, IO), a concurrent request's with_scope mutates the same scope stack and scope/transaction/breadcrumbs/user leak between requests -- reproduced as 5/6 events attributed to the wrong user, including their email (cross-user PII bleed). Add an opt-in config.isolation_level (:thread default, :fiber) that routes the current hub through Sentry::HubStorage. :fiber uses Ruby 3.2+ Fiber Storage (Fiber[]), which gives per-request isolation AND child-fiber inheritance, so it fixes Falcon without re-introducing the graphql-ruby context loss that #1374/#1380 addressed (old fiber-local vars did not inherit; Fiber Storage does). Requesting :fiber on Ruby < 3.2 warns and falls back to :thread. Default :thread is byte-for-byte the previous behaviour. Mirrors Rails' config.active_support.isolation_level. Refs #1495. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(rails): route ActiveJob hub save/restore through HubStorage The ActiveJob integration saved and restored the surrounding hub with Thread.current.thread_variable_get/set(Sentry::THREAD_LOCAL) directly, which is wrong under config.isolation_level = :fiber (it would read/write thread storage while the active hub lives in fiber storage). Route both sites through Sentry::HubStorage so the save/restore follows the configured isolation level. Behaviour is byte-for-byte unchanged under the default :thread level. Verified with a real ActiveJob perform_now: under :thread the outer hub is restored and the job's user does not leak; under :fiber, concurrent jobs run as sibling fibers each keep their own user. Refs #1495. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix: sync HubStorage on post-init isolation_level change; stabilize specs Two follow-ups from review: - Configuration#isolation_level= now applies the level to HubStorage when the SDK is already initialized, so changing Sentry.configuration.isolation_level after Sentry.init takes effect instead of leaving the SDK on the old backend. The default is assigned to the ivar directly in initialize so a throwaway Configuration.new cannot clobber the active isolation level (only Sentry.init builds the live config). - Make the :fiber isolation_level specs deterministic across Ruby versions by stubbing fiber_storage_available?, and add an explicit downgrade-to-:thread spec. Previously these asserted :fiber unconditionally and would fail on the Ruby < 3.2 CI cells, where :fiber correctly falls back to :thread. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * simplify, remove HubStorage, add internal apis --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Co-authored-by: Neel Shah <neel.shah@sentry.io>
1 parent b625f46 commit 7ab412f

7 files changed

Lines changed: 185 additions & 16 deletions

File tree

sentry-rails/lib/sentry/rails/active_job.rb

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -117,13 +117,7 @@ def log_producer_span_error(e)
117117
end
118118

119119
def record(job, trace_headers: nil, user: nil, &block)
120-
# Always give this thread a fresh hub cloned from the main hub so
121-
# the job's events are fully isolated. Save and restore whatever
122-
# hub was on the thread before (e.g. the Rack request hub set by
123-
# CaptureExceptions, or a stale hub left by a recycled thread-pool
124-
# thread) so the outer context continues working correctly after
125-
# the job finishes.
126-
original_hub = Thread.current.thread_variable_get(Sentry::THREAD_LOCAL)
120+
original_hub = Sentry.get_current_hub_internal
127121
Sentry.clone_hub_to_current_thread
128122

129123
Sentry.with_scope do |scope|
@@ -171,7 +165,7 @@ def record(job, trace_headers: nil, user: nil, &block)
171165
end
172166
end
173167
ensure
174-
Thread.current.thread_variable_set(Sentry::THREAD_LOCAL, original_hub)
168+
Sentry.set_current_hub_internal(original_hub)
175169
end
176170

177171
def set_messaging_data(transaction, job)

sentry-ruby/lib/sentry-ruby.rb

Lines changed: 37 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -271,7 +271,8 @@ def init(&block)
271271
client = Client.new(config)
272272
scope = Scope.new(max_breadcrumbs: config.max_breadcrumbs)
273273
hub = Hub.new(client, scope)
274-
Thread.current.thread_variable_set(THREAD_LOCAL, hub)
274+
@hub_isolation_level = config.hub_isolation_level
275+
set_current_hub_internal(hub)
275276
@main_hub = hub
276277
@background_worker = Sentry::BackgroundWorker.new(config)
277278
@session_flusher = config.session_tracking? ? Sentry::SessionFlusher.new(config, client) : nil
@@ -309,7 +310,7 @@ def close
309310

310311
MUTEX.synchronize do
311312
@main_hub = nil
312-
Thread.current.thread_variable_set(THREAD_LOCAL, nil)
313+
set_current_hub_internal(nil)
313314
end
314315
end
315316

@@ -362,7 +363,7 @@ def get_current_hub
362363
# ideally, we should do this proactively whenever a new thread is created
363364
# but it's impossible for the SDK to keep track every new thread
364365
# so we need to use this rather passive way to make sure the app doesn't crash
365-
Thread.current.thread_variable_get(THREAD_LOCAL) || clone_hub_to_current_thread
366+
get_current_hub_internal || clone_hub_to_current_thread
366367
end
367368

368369
# Returns the current active client.
@@ -380,12 +381,14 @@ def get_current_scope
380381
get_current_hub.current_scope
381382
end
382383

383-
# Clones the main thread's active hub and stores it to the current thread.
384+
# Clones the main hub and stores it for the current execution context
385+
# (the current thread, or the current fiber when +config.hub_isolation_level+
386+
# is +:fiber+).
384387
#
385388
# @return [void]
386389
def clone_hub_to_current_thread
387390
return unless initialized?
388-
Thread.current.thread_variable_set(THREAD_LOCAL, get_main_hub.clone)
391+
set_current_hub_internal(get_main_hub.clone)
389392
end
390393

391394
# Takes a block and yields the current active scope.
@@ -737,6 +740,35 @@ def utc_now
737740
def dependency_installed?(name)
738741
Object.const_defined?(name)
739742
end
743+
744+
# Reads the hub stored for the current execution context. The active
745+
# isolation level (cached from +config.hub_isolation_level+ at init) decides
746+
# whether that context is the current thread or the current fiber. Reads the
747+
# cached level rather than the configuration to avoid recursing back through
748+
# hub resolution.
749+
#
750+
# @!visibility private
751+
# @return [Hub, nil]
752+
def get_current_hub_internal
753+
if @hub_isolation_level == :fiber
754+
::Fiber[THREAD_LOCAL]
755+
else
756+
::Thread.current.thread_variable_get(THREAD_LOCAL)
757+
end
758+
end
759+
760+
# Stores +hub+ for the current execution context (thread or fiber).
761+
#
762+
# @!visibility private
763+
# @param hub [Hub, nil]
764+
# @return [Hub, nil]
765+
def set_current_hub_internal(hub)
766+
if @hub_isolation_level == :fiber
767+
::Fiber[THREAD_LOCAL] = hub
768+
else
769+
::Thread.current.thread_variable_set(THREAD_LOCAL, hub)
770+
end
771+
end
740772
end
741773
end
742774

sentry-ruby/lib/sentry/configuration.rb

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -389,6 +389,23 @@ class Configuration
389389
# @return [Boolean]
390390
attr_accessor :strict_trace_continuation
391391

392+
# Which execution primitive owns the SDK's current hub.
393+
#
394+
# [+:thread+ (default)] Store the hub in thread-local storage. Correct for
395+
# thread-based servers (Puma, Unicorn) and background processors (Sidekiq,
396+
# Resque). Every fiber on a thread shares one hub.
397+
# [+:fiber+] Store the hub in Fiber Storage (Ruby 3.2+). Each fiber gets its
398+
# own hub and child fibers inherit it, so concurrent requests on a
399+
# fiber-based server (Falcon/async) are isolated instead of sharing and
400+
# corrupting one another's scope. Requested on a Ruby without Fiber
401+
# Storage (< 3.2), the SDK logs a warning and falls back to +:thread+.
402+
#
403+
# @return [Symbol]
404+
attr_reader :hub_isolation_level
405+
406+
# Isolation levels the SDK understands for hub storage.
407+
ISOLATION_LEVELS = %i[thread fiber].freeze
408+
392409
# these are not config options
393410
# @!visibility private
394411
attr_reader :errors, :gem_specs
@@ -541,6 +558,7 @@ def initialize
541558
self.capture_queue_time = true
542559
self.org_id = nil
543560
self.strict_trace_continuation = false
561+
self.hub_isolation_level = :thread
544562

545563
spotlight_env = ENV["SENTRY_SPOTLIGHT"]
546564
spotlight_bool = Sentry::Utils::EnvHelper.env_to_bool(spotlight_env, strict: true)
@@ -610,6 +628,23 @@ def release=(value)
610628
@release = value
611629
end
612630

631+
def hub_isolation_level=(level)
632+
level = level.to_sym if level.respond_to?(:to_sym)
633+
634+
unless ISOLATION_LEVELS.include?(level)
635+
raise ArgumentError, "hub_isolation_level must be one of #{ISOLATION_LEVELS.inspect}, got #{level.inspect}"
636+
end
637+
638+
# :fiber relies on Fiber Storage (Ruby 3.2+); downgrade so the hub is never
639+
# asked to call Fiber[] on a Ruby that lacks it.
640+
if level == :fiber && !fiber_storage_available?
641+
log_warn("hub_isolation_level :fiber requires Ruby 3.2+ Fiber Storage; falling back to :thread on Ruby #{RUBY_VERSION}.")
642+
level = :thread
643+
end
644+
645+
@hub_isolation_level = level
646+
end
647+
613648
def breadcrumbs_logger=(logger)
614649
loggers =
615650
if logger.is_a?(Array)
@@ -806,6 +841,10 @@ def run_after_close_callbacks
806841

807842
private
808843

844+
def fiber_storage_available?
845+
::Fiber.respond_to?(:[]) && ::Fiber.respond_to?(:[]=)
846+
end
847+
809848
def init_dsn(dsn_string)
810849
return if dsn_string.nil? || dsn_string.empty?
811850

sentry-ruby/lib/sentry/test_helper.rb

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,10 +52,10 @@ def setup_sentry_test(&block)
5252
test_client = Sentry::Client.new(dummy_config.dup)
5353
main_hub.bind_client(test_client)
5454

55-
# Realign the current thread's hub with the main hub so direct
55+
# Realign the current execution context's hub with the main hub so direct
5656
# `sentry_events` reads and any hub the Rack middleware clones from the
5757
# main hub all observe the same DummyTransport.
58-
Thread.current.thread_variable_set(Sentry::THREAD_LOCAL, main_hub)
58+
Sentry.set_current_hub_internal(main_hub)
5959
end
6060

6161
# Clears all stored events and envelopes.
@@ -165,7 +165,7 @@ def reset_sentry_globals!
165165
Sentry.instance_variable_set(:"@#{var}", nil)
166166
end
167167

168-
Thread.current.thread_variable_set(Sentry::THREAD_LOCAL, nil)
168+
Sentry.set_current_hub_internal(nil)
169169
end
170170
end
171171
end

sentry-ruby/spec/sentry/configuration_spec.rb

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -696,6 +696,42 @@ class SentryConfigurationSample < Sentry::Configuration
696696
end
697697
end
698698

699+
describe "#hub_isolation_level" do
700+
it "defaults to :thread" do
701+
expect(subject.hub_isolation_level).to eq(:thread)
702+
end
703+
704+
it "accepts :thread" do
705+
subject.hub_isolation_level = :thread
706+
expect(subject.hub_isolation_level).to eq(:thread)
707+
end
708+
709+
it "raises ArgumentError for an unknown level" do
710+
expect { subject.hub_isolation_level = :process }
711+
.to raise_error(ArgumentError, /hub_isolation_level must be one of/)
712+
end
713+
714+
context "when Fiber storage is available", when: { fiber_storage?: [] } do
715+
it "accepts :fiber" do
716+
subject.hub_isolation_level = :fiber
717+
expect(subject.hub_isolation_level).to eq(:fiber)
718+
end
719+
720+
it "coerces string values" do
721+
subject.hub_isolation_level = "fiber"
722+
expect(subject.hub_isolation_level).to eq(:fiber)
723+
end
724+
end
725+
726+
context "when Fiber storage is unavailable", when: { no_fiber_storage?: [] } do
727+
it "falls back to :thread with a warning" do
728+
expect(subject).to receive(:log_warn).with(/requires Ruby 3\.2\+ Fiber Storage/)
729+
subject.hub_isolation_level = :fiber
730+
expect(subject.hub_isolation_level).to eq(:thread)
731+
end
732+
end
733+
end
734+
699735
describe "#validate" do
700736
it "logs a warning if StackProf is not installed" do
701737
allow(Sentry).to receive(:dependency_installed?).with(:StackProf).and_return(false)

sentry-ruby/spec/sentry_spec.rb

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,66 @@
110110
end
111111
end
112112

113+
describe "fiber isolation", when: { fiber_storage?: [] } do
114+
before do
115+
perform_basic_setup { |config| config.hub_isolation_level = :fiber }
116+
end
117+
118+
after do
119+
described_class.set_current_hub_internal(nil)
120+
described_class.instance_variable_set(:@hub_isolation_level, :thread)
121+
end
122+
123+
# Regression for cross-request contamination on fiber-based servers (Falcon,
124+
# async), where many concurrent requests run as sibling fibers on one thread.
125+
# With thread-local storage they share a single hub, so a scope opened by one
126+
# request and held across a reactor yield is visible to (and clobbered by) the
127+
# others. Fiber storage gives each request fiber its own hub.
128+
it "keeps each sibling fiber's scope isolated across a yield" do
129+
transport = described_class.get_main_hub.current_client.transport
130+
transport.events.clear
131+
132+
requests = 3.times.map do |i|
133+
Fiber.new do
134+
described_class.clone_hub_to_current_thread
135+
described_class.configure_scope { |scope| scope.set_user(id: i) }
136+
Fiber.yield # simulate yielding to the reactor mid-request
137+
described_class.capture_message(i.to_s)
138+
end
139+
end
140+
141+
requests.each(&:resume) # every request sets its user, then yields
142+
requests.each(&:resume) # every request now captures its event
143+
144+
attributed = transport.events.to_h { |e| [e.message.to_i, e.user[:id]] }
145+
expect(attributed).to eq({ 0 => 0, 1 => 1, 2 => 2 })
146+
end
147+
148+
it "lets a child fiber inherit the parent request's hub" do
149+
described_class.clone_hub_to_current_thread
150+
parent_hub = described_class.get_current_hub
151+
152+
inherited = Fiber.new { described_class.get_current_hub }.resume
153+
154+
expect(inherited).to eq(parent_hub)
155+
end
156+
157+
it "stores the hub in a fiber variable (instead of a thread variable)" do
158+
described_class.set_tags(outside_fiber: true)
159+
160+
fiber = Fiber.new do
161+
described_class.clone_hub_to_current_thread
162+
described_class.set_tags(inside_fiber: true)
163+
described_class.get_current_scope.tags
164+
end
165+
166+
inside_tags = fiber.resume
167+
168+
expect(inside_tags).to eq({ outside_fiber: true, inside_fiber: true })
169+
expect(described_class.get_current_scope.tags).to eq({ outside_fiber: true })
170+
end
171+
end
172+
113173
shared_examples "capture_helper" do
114174
context "with sending_allowed? condition" do
115175
before do

sentry-ruby/spec/spec_helper.rb

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,14 @@ def self.ruby_version?(op, version)
125125
RUBY_VERSION.public_send(op, version)
126126
end
127127

128+
def self.fiber_storage?
129+
Fiber.respond_to?(:[]) && Fiber.respond_to?(:[]=)
130+
end
131+
132+
def self.no_fiber_storage?
133+
!fiber_storage?
134+
end
135+
128136
def self.ruby_engine?(engine)
129137
RUBY_ENGINE == engine
130138
end

0 commit comments

Comments
 (0)