Skip to content

Fix race condition in Subscription#verify!#65

Open
bellisabell wants to merge 1 commit intomainfrom
bell/fix-verify-race-condition
Open

Fix race condition in Subscription#verify!#65
bellisabell wants to merge 1 commit intomainfrom
bell/fix-verify-race-condition

Conversation

@bellisabell
Copy link
Copy Markdown
Contributor

Problem

The verify! method in Subscription has a race condition that can send duplicate new subscriber notifications.

When two concurrent requests call verify! on the same subscription:

  1. Both check verified_at.present? → both see nil
  2. Both set verified_at and save
  3. Both call send_new_subscriber_notification

Solution

Wrap the verify! method in with_lock to use pessimistic database row-level locking. This ensures only one process can read and modify the record at a time, making the check-then-act pattern atomic.

def verify!
  with_lock do
    already_verified = verified_at.present?
    self.verified_at = Time.zone.now
    self.unsubscribed_at = nil
    save!

    send_new_subscriber_notification unless already_verified
  end
end

Closes #29

Wrap verify! in with_lock to prevent concurrent execution that could
send duplicate new subscriber notifications.

The race condition occurred when two concurrent requests both checked
verified_at.present? before either had saved, causing both to send
notifications.

Closes #29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: Race condition in Subscription#verify! can send duplicate notifications

2 participants