Rosa Gutierrez
95114b1d67
When concurrent NotifyRecipientsJobs process an Event and a Mention for
the same user+card, Rails' dirty tracking can skip writing source_type
in the UPDATE if it hasn't changed from the stale in-memory value,
leaving source_type and source_id mismatched (e.g. source_type='Event'
with a Mention's source_id, resulting in a nil source).
Force source_type to always be included in the UPDATE via
source_type_will_change! to prevent this.
Here's a sample timeline of this race condition happening in the real
world (with simplified IDs):
Two `NotifyRecipientsJob` involving notification `03fklpu`, same card,
same user, same comment — enqueued within 50ms of each other:
1. Both jobs load notification `03fklpu` — it has source_type='Mention'
(from a previous job)
2. `EventNotifier` writes at 21:22:36.051: ```sql SET
source_type='Event', source_id=<event_id>, unread_count=1 ```
3. `source_type` included because it changed ('Mention' → 'Event')
4. `MentionNotifier` writes at 21:22:36.057 (~6ms later): ```sql SET
source_id=<mention_id>, unread_count=1 ```
5. No `source_type`! It was `'Mention'` when loaded and `'Mention'` is
what it's setting → not dirty → skipped
6. Final DB state: `source_type='Event'` (from step 2, untouched),
`source_id=<mention_id>` (from step 3)
`Notification.source` now does `Event.find(<mention_id>) → nil.`
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
49 lines
1.4 KiB
Ruby
49 lines
1.4 KiB
Ruby
class Notifier
|
|
attr_reader :source
|
|
|
|
class << self
|
|
def for(source)
|
|
case source
|
|
when Event
|
|
"Notifier::#{source.eventable.class}EventNotifier".safe_constantize&.new(source)
|
|
when Mention
|
|
MentionNotifier.new(source)
|
|
end
|
|
end
|
|
end
|
|
|
|
def notify
|
|
if should_notify?
|
|
# Processing recipients in order avoids deadlocks if notifications overlap.
|
|
recipients.sort_by(&:id).map do |recipient|
|
|
notification = Notification.create_or_find_by(user: recipient, card: source.card) do |n|
|
|
n.source = source
|
|
n.creator = creator
|
|
n.unread_count = 1
|
|
end
|
|
|
|
unless notification.previously_new_record?
|
|
# Always include source_type in the update to prevent a race condition between
|
|
# concurrent Event and Mention notifier jobs: without this, Rails' dirty tracking
|
|
# may skip source_type when it hasn't changed from the stale in-memory value,
|
|
# even though another job has since modified it in the database, leaving
|
|
# source_type and source_id mismatched.
|
|
notification.source_type_will_change!
|
|
notification.update!(source: source, creator: creator, read_at: nil, unread_count: notification.unread_count + 1)
|
|
end
|
|
|
|
notification
|
|
end
|
|
end
|
|
end
|
|
|
|
private
|
|
def initialize(source)
|
|
@source = source
|
|
end
|
|
|
|
def should_notify?
|
|
!creator.system?
|
|
end
|
|
end
|