Is this Rails validation thread-safe

Question

This is running in multiple Sidekiq instances and workers at the same time and it seems that is has generated a couple of issues, like instances getting assigned the "It was alerted recently" error when shouldn't and the opposite.

It is rare, but it is happening, is this the problem or maybe it is something else?

class BrokenModel < ActiveRecord::Base
  validates_with BrokenValidator

end

class BrokenValidator < ActiveModel::Validator
  def validate record
    @record = record

    check_alerted
  end

  private

  def check_alerted
    if AtomicGlobalAlerted.new(@record).valid?
      @record.errors[:base] << "It was alerted recently"
    end

    p "check_alerted: #{@record.errors[:base]}"
  end
end

class AtomicGlobalAlerted
  include Redis::Objects
  attr_accessor :id

  def initialize id
    @id = id
    @fredis = nil

    Sidekiq.redis do |redis|
        @fredis = FreshRedis.new(redis, freshness: 7.days, granularity: 4.hours)
    end
  end

  def valid?
    @fredis.smembers.includes?(@id)
  end
end

Answer 1

We were experiencing something similar at work and after A LOT of digging finally figured out what was happening.

The class method validates_with uses one instance of the validator (BrokenValidator) to validate all instances of the class you're trying to validate (BrokenModel). Normally this is fine but you are assigning a variable (@record) and accessing that variable in another method (check_alerted) so other threads are assigning @record while other threads are still trying to check_alerted.

There are two ways you can fix this:

1) Pass record to check_alerted:

class BrokenValidator < ActiveModel::Validator
  def validate(record)    
    check_alerted(record)
  end

  private

  def check_alerted(record)
    if AtomicGlobalAlerted.new(record).valid?
      record.errors[:base] << "It was alerted recently"
    end

    p "check_alerted: #{record.errors[:base]}"
  end
end

2) Use the instance version of validates_with which makes a new validator instance for each model instance you want to validate:

class BrokenModel < ActiveRecord::Base
  validate :instance_validators

  def instance_validators
    validates_with BrokenValidator
  end

end

Either solution should work and solve the concurrency problem. Let me know if you experience any other issues.