Concurrency issue in MemoryCache

[madejejej]

Hi!

We're running DeviceDetector in Sidekiq and under high load we sometimes are able to get NoMethodError: undefined method '[]' for nil:NilClass from:

"/webapps/api/vendor/bundle/ruby/2.6.0/gems/device_detector-1.0.1/lib/device_detector/os.rb" line 11 in short_name
"/webapps/api/vendor/bundle/ruby/2.6.0/gems/device_detector-1.0.1/lib/device_detector.rb" line 66 in device_type

That's basically a MemoryCache#get_or_set call which returns nil.

Here's the code for reference:

device_detector/lib/device_detector/memory_cache.rb

Lines 30 to 39 in f30a385

	def get_or_set(key, value = nil)
	string_key = String(key)
	if key?(string_key)
	get(string_key)
	else
	value = yield if block_given?
	set(string_key, value)
	end
	end

My belief what happens is:

1. The cache is full and it will be purged on the next write
2. Thread 1 In line 33 we check if the given key K1 exists
3. Thread 2 in the meantime calls cache.set('k2', 1) and purges the cache. K1 was an old key and got purged
4. Thread 1 tries to get(K1) and returns, but there is no such key now so it returns nil

Here's a script that can replicate it:

require 'device_detector'

cache = DeviceDetector::MemoryCache.new(max_cache_keys: 3)

# fill the cache 100% capacity
cache.set(1, 1)
cache.set(2, 2)
cache.set(3, 3)

# now on, the oldest keys should be purged on each write

sampler = (1..4).to_a

threads = 20.times.map do |thread_id|
  Thread.new do
    1_000_000.times do |i|
      val = sampler.sample

      ret = cache.get_or_set(val, val)

      puts "[Thread #{thread_id}] BOOM" if ret.nil?
    end
  end
end

threads.each(&:join)

A simple solution would be to remove the check key?(string_key) and just get the value and return it if it's non-nil.