From 7bc2dbb31591cd7dd85c9b9c4aebc099d9c31406 Mon Sep 17 00:00:00 2001 From: Sean Floyd Date: Sat, 25 Jul 2026 21:56:37 -0400 Subject: [PATCH 1/2] fix: isolate hubs inherited through fiber storage Fiber storage is inherited by child fibers and by threads started from them, handing over the same Hub object rather than a copy. Under hub_isolation_level = :fiber that left sibling request fibers and worker threads sharing one hub, and one unsynchronized scope stack, so concurrent requests and jobs read each other's tags. The `|| clone` fallback never fired because the inherited value was not nil. Tag each stored entry with its owning fiber so a context that inherited one takes its own copy. The copy keeps the live span: Scope#dup deep-copies it, and spans recorded on a detached transaction copy are never sent, which would break the Async fan-out this mode exists for. Reported by @trevorturk in #3018. Co-Authored-By: Claude Opus 5 (1M context) --- sentry-ruby/lib/sentry-ruby.rb | 28 +++++++++- sentry-ruby/lib/sentry/configuration.rb | 7 ++- sentry-ruby/spec/sentry_spec.rb | 74 ++++++++++++++++++++++--- 3 files changed, 96 insertions(+), 13 deletions(-) diff --git a/sentry-ruby/lib/sentry-ruby.rb b/sentry-ruby/lib/sentry-ruby.rb index 83d075320..b9c072629 100644 --- a/sentry-ruby/lib/sentry-ruby.rb +++ b/sentry-ruby/lib/sentry-ruby.rb @@ -751,12 +751,35 @@ def dependency_installed?(name) # @return [Hub, nil] def get_current_hub_internal if @hub_isolation_level == :fiber - ::Fiber[THREAD_LOCAL] + owner, hub = ::Fiber[THREAD_LOCAL] + + # Child fibers, and threads started from them, inherit fiber storage + # holding the same Hub object rather than a copy. + if owner.equal?(::Fiber.current) + hub + elsif hub + set_current_hub_internal(fork_hub(hub)) + end else ::Thread.current.thread_variable_get(THREAD_LOCAL) end end + # Copies +hub+ for a context that inherited it. The span is re-attached + # because Scope#dup deep-copies it, and spans recorded on a detached + # transaction copy are never sent. + # + # @!visibility private + # @param hub [Hub] + # @return [Hub] + def fork_hub(hub) + span = hub.current_scope.span + forked = hub.clone + forked.current_scope.set_span(span) if span + + forked + end + # Stores +hub+ for the current execution context (thread or fiber). # # @!visibility private @@ -764,7 +787,8 @@ def get_current_hub_internal # @return [Hub, nil] def set_current_hub_internal(hub) if @hub_isolation_level == :fiber - ::Fiber[THREAD_LOCAL] = hub + ::Fiber[THREAD_LOCAL] = [::Fiber.current, hub] + hub # callers use the return value, so don't return the pair else ::Thread.current.thread_variable_set(THREAD_LOCAL, hub) end diff --git a/sentry-ruby/lib/sentry/configuration.rb b/sentry-ruby/lib/sentry/configuration.rb index 1f29d1854..6c1f23527 100644 --- a/sentry-ruby/lib/sentry/configuration.rb +++ b/sentry-ruby/lib/sentry/configuration.rb @@ -395,9 +395,10 @@ class Configuration # 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 + # own hub, so concurrent requests on a fiber-based server (Falcon/async) + # are isolated instead of sharing and corrupting one another's scope. A + # fiber or thread started from another inherits its context and trace, but + # its own scope changes stay local. Requested on a Ruby without Fiber # Storage (< 3.2), the SDK logs a warning and falls back to +:thread+. # # @return [Symbol] diff --git a/sentry-ruby/spec/sentry_spec.rb b/sentry-ruby/spec/sentry_spec.rb index 0b0403063..d88aed57c 100644 --- a/sentry-ruby/spec/sentry_spec.rb +++ b/sentry-ruby/spec/sentry_spec.rb @@ -121,17 +121,16 @@ 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. + # async), where concurrent requests are sibling fibers on one thread: a scope + # held across a reactor yield must not be visible to the others. it "keeps each sibling fiber's scope isolated across a yield" do transport = described_class.get_main_hub.current_client.transport transport.events.clear + # Deliberately no clone_hub_to_current_thread: real request fibers never + # call it, so the implicit path is the one that has to isolate. 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) @@ -145,13 +144,72 @@ expect(attributed).to eq({ 0 => 0, 1 => 1, 2 => 2 }) end - it "lets a child fiber inherit the parent request's hub" do + it "gives a child fiber the parent's context in its own hub" do described_class.clone_hub_to_current_thread + described_class.set_tags(request: "req-1") parent_hub = described_class.get_current_hub - inherited = Fiber.new { described_class.get_current_hub }.resume + child_tags, child_hub = Fiber.new do + described_class.set_tags(subtask: true) + [described_class.get_current_scope.tags, described_class.get_current_hub] + end.resume - expect(inherited).to eq(parent_hub) + expect(child_tags).to eq({ request: "req-1", subtask: true }) + expect(child_hub).not_to be(parent_hub) + expect(described_class.get_current_scope.tags).to eq({ request: "req-1" }) + end + + # Fiber storage is inherited by Thread.new too, so a worker pool started + # after anything seeded a hub would otherwise share it across every worker. + it "gives a thread started from a fiber its own hub" do + described_class.clone_hub_to_current_thread + parent_hub = described_class.get_current_hub + + hubs = 4.times.map { Thread.new { described_class.get_current_hub } }.map(&:value) + + expect(hubs.uniq.size).to eq(4) + expect(hubs).not_to include(parent_hub) + end + + # Isolating the scope must not detach the trace: on Async servers the + # fan-out inside a request is exactly what we need spans for. + it "records a child fiber's spans on the parent's transaction" do + perform_basic_setup do |config| + config.hub_isolation_level = :fiber + config.traces_sample_rate = 1.0 + end + + transaction = described_class.start_transaction(name: "req", op: "http.server") + described_class.get_current_scope.set_span(transaction) + + Fiber.new { described_class.with_child_span(op: "child.in.fiber") { } }.resume + Thread.new { described_class.with_child_span(op: "child.in.thread") { } }.join + transaction.finish + + expect(transaction.span_recorder.spans.map(&:op)) + .to eq(["http.server", "child.in.fiber", "child.in.thread"]) + end + + it "keeps concurrent threads' with_scope blocks from seeing each other's tags" do + described_class.clone_hub_to_current_thread + ready = Queue.new + gate = Queue.new + + threads = 4.times.map do |i| + Thread.new do + described_class.with_scope do |scope| + scope.set_tags(job: i) + ready << :in + gate.pop # hold every job open inside its own scope + described_class.get_current_scope.tags[:job] + end + end + end + + 4.times { ready.pop } + 4.times { gate << :go } + + expect(threads.map(&:value)).to eq([0, 1, 2, 3]) end it "stores the hub in a fiber variable (instead of a thread variable)" do From 6eaf8c858930997c8cc497e72c9f08f894fde790 Mon Sep 17 00:00:00 2001 From: Sean Floyd Date: Sat, 25 Jul 2026 22:28:13 -0400 Subject: [PATCH 2/2] test: scope the span spec to fibers The fiber-storage gem polyfills Fiber[] on Ruby < 3.2 and on JRuby, so :fiber is active there too, but the polyfill does not inherit into Thread.new. A thread started from a fiber therefore begins from the main hub and cannot stay on the parent's trace, which the thread half of this expectation assumed it could. Co-Authored-By: Claude Opus 5 (1M context) --- sentry-ruby/spec/sentry_spec.rb | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/sentry-ruby/spec/sentry_spec.rb b/sentry-ruby/spec/sentry_spec.rb index d88aed57c..9c5751cf1 100644 --- a/sentry-ruby/spec/sentry_spec.rb +++ b/sentry-ruby/spec/sentry_spec.rb @@ -182,12 +182,13 @@ transaction = described_class.start_transaction(name: "req", op: "http.server") described_class.get_current_scope.set_span(transaction) + # Threads are deliberately not covered here: the fiber-storage polyfill + # used on Ruby < 3.2 does not inherit into Thread.new, so a thread there + # starts from the main hub and cannot stay on this trace. Fiber.new { described_class.with_child_span(op: "child.in.fiber") { } }.resume - Thread.new { described_class.with_child_span(op: "child.in.thread") { } }.join transaction.finish - expect(transaction.span_recorder.spans.map(&:op)) - .to eq(["http.server", "child.in.fiber", "child.in.thread"]) + expect(transaction.span_recorder.spans.map(&:op)).to eq(["http.server", "child.in.fiber"]) end it "keeps concurrent threads' with_scope blocks from seeing each other's tags" do