Root cause
lib/track_changes/attribution.rb:2:
mattr_accessor :default_attribution
That's a single @@default_attribution shared by every thread in the process. with_changes_attributed_to sets it, yields, and restores it in an ensure:
def self.with_changes_attributed_to(new_source, &block)
old_source = self.default_attribution
self.default_attribution = new_source
block.call
ensure
self.default_attribution = old_source
end
Model#track_changes_by falls back to it (lib/track_changes/model.rb:61), and attribute_changes_to (lib/track_changes/controller.rb) wraps the whole thing in an around_action.
So in a threaded server, the "current" attribution is process-wide, not per-request. Two concurrent requests interleave like this:
- Thread A (Alice) sets
default_attribution = alice
- Thread B (Bob) sets
default_attribution = bob
- Thread A saves a tracked record → reads
default_attribution → records the change as Bob's
- Thread B finishes, restores to
alice; thread A finishes, restores to nil
This isn't theoretical for the consuming apps: stolo_connect runs Puma with 5 threads in a single process (config/puma.rb:8, workers commented out), and its referral/site/payment controllers all use attribute_changes_to :current_user. Any two overlapping requests from different users touching tracked records can cross-attribute.
The window is narrow (the misattribution has to land between another thread's set and restore) which is probably why it hasn't been noticed — but the failure is silent and writes a permanently wrong audit row. For a change-tracking gem specifically, "who made this change" being occasionally wrong is worse than it being absent, because the record looks authoritative.
The ensure block makes this worse in one respect: it restores to whatever old_source that thread captured, so threads can also clobber each other's restore, leaving attribution set to a stale user after a request completes.
Suggested resolution
Back the attribution with ActiveSupport::CurrentAttributes, which is per-thread (and per-fiber) and already resets between requests via the executor. Consuming apps are on Rails 8 and already use this idiom (Current.user in stolo_connect).
Rough shape:
module TrackChanges
class Current < ActiveSupport::CurrentAttributes
attribute :default_attribution
end
def self.default_attribution
Current.default_attribution
end
def self.default_attribution=(value)
Current.default_attribution = value
end
end
That keeps with_changes_attributed_to and the mattr-style read/write API intact, so no consumer changes.
One constraint: ActiveSupport::CurrentAttributes arrived in Rails 5.2, but the gemspec declares rails >= 4.2.0 (track_changes.gemspec:20). Adopting it means raising that floor. Worth checking whether anything still consuming this gem is below 5.2 — if something is, the alternative is a plain Thread.current-keyed store, which works everywhere but doesn't get the executor's automatic reset between requests.
Worth a regression spec that runs two threads with different attributions concurrently and asserts each one's diff gets its own user.
Context
Found while scoping change tracking for guardian reports in stolo_connect.
Refs https://github.com/culturecode/stolo_connect/issues/TICKET1
Root cause
lib/track_changes/attribution.rb:2:That's a single
@@default_attributionshared by every thread in the process.with_changes_attributed_tosets it, yields, and restores it in anensure:Model#track_changes_byfalls back to it (lib/track_changes/model.rb:61), andattribute_changes_to(lib/track_changes/controller.rb) wraps the whole thing in anaround_action.So in a threaded server, the "current" attribution is process-wide, not per-request. Two concurrent requests interleave like this:
default_attribution = alicedefault_attribution = bobdefault_attribution→ records the change as Bob'salice; thread A finishes, restores tonilThis isn't theoretical for the consuming apps:
stolo_connectruns Puma with 5 threads in a single process (config/puma.rb:8,workerscommented out), and its referral/site/payment controllers all useattribute_changes_to :current_user. Any two overlapping requests from different users touching tracked records can cross-attribute.The window is narrow (the misattribution has to land between another thread's set and restore) which is probably why it hasn't been noticed — but the failure is silent and writes a permanently wrong audit row. For a change-tracking gem specifically, "who made this change" being occasionally wrong is worse than it being absent, because the record looks authoritative.
The
ensureblock makes this worse in one respect: it restores to whateverold_sourcethat thread captured, so threads can also clobber each other's restore, leaving attribution set to a stale user after a request completes.Suggested resolution
Back the attribution with
ActiveSupport::CurrentAttributes, which is per-thread (and per-fiber) and already resets between requests via the executor. Consuming apps are on Rails 8 and already use this idiom (Current.userinstolo_connect).Rough shape:
That keeps
with_changes_attributed_toand themattr-style read/write API intact, so no consumer changes.One constraint:
ActiveSupport::CurrentAttributesarrived in Rails 5.2, but the gemspec declaresrails >= 4.2.0(track_changes.gemspec:20). Adopting it means raising that floor. Worth checking whether anything still consuming this gem is below 5.2 — if something is, the alternative is a plainThread.current-keyed store, which works everywhere but doesn't get the executor's automatic reset between requests.Worth a regression spec that runs two threads with different attributions concurrently and asserts each one's diff gets its own user.
Context
Found while scoping change tracking for guardian reports in
stolo_connect.Refs https://github.com/culturecode/stolo_connect/issues/TICKET1