From 9c580dbf15fbe4c0b479204604a42d2cc6d40286 Mon Sep 17 00:00:00 2001 From: Jon Calvert Date: Tue, 5 Jan 2016 11:20:43 -0500 Subject: [PATCH] Add a failing, somewhat contrived spec to demonstrate EXISTS keys being orphaned. We share redis clients between threads (redis obj is threadsafe) and because of this the 2nd thread will be blocking, waiting for the AVAILABLE key and thus the first thread is unable to finish cleanup. Eventually the AVAILABLE key expires but the EXISTS key does not. The failure to succeed with the signal() call could be due to loss of network connection, process crash, etc. Leaving an unexpiring EXISTS key but no AVAILABLE key means that a retry will believe the semaphore exists and wait for the AVAILABLE which never comes. --- spec/semaphore_spec.rb | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/spec/semaphore_spec.rb b/spec/semaphore_spec.rb index 45f5ff4..e024d47 100644 --- a/spec/semaphore_spec.rb +++ b/spec/semaphore_spec.rb @@ -148,6 +148,31 @@ sleep 3.0 expect(@redis.keys.count).to eq(original_key_size) end + + it "does not leave an exists key without an expiration when an expiration is specified" do + original_key_size = @redis.keys.count + queue = Queue.new + threads = 2.times.collect do + Thread.new do + Redis::Semaphore.new(:my_semaphore, :redis => @redis, :expiration => 3).lock(5) do + sleep 1 + end + end + end + sleep 4.0 + @redis2 = Redis.new :db => 15 + threads.each{|t| t.kill } #ensure signal step fails + expect(@redis2.ttl("SEMAPHORE:my_semaphore:EXISTS").to_s).to_not eql("-1") + sleep 4.0 #allow blpop timeout to occur + thrd = Thread.new do + Redis::Semaphore.new(:my_semaphore, :redis => @redis2, :expiration => 3).lock(5) do + queue << "work" + end + end + thrd.join(3) + expect(queue.size).to eql(1) + expect(@redis.ttl("SEMAPHORE:my_semaphore:EXISTS").to_s).to_not eql("-1") + end end describe "semaphore without staleness checking" do