Remove the extraneous expire during create, since the subsequent persist will obviate it anyway and since keys can expire during a MULTI transaction anyway. Return the removed statement where we always reset the key expiration prior to attempting to obtain a lock. This prevents the orphaned EXISTS key that never expires.

Author: Jon Calvert

lib/redis/semaphore.rb

@@ -36,7 +36,7 @@ def exists_or_create!
         if token == API_VERSION && @redis.get(version_key).nil?
           @redis.set(version_key, API_VERSION)
         end
-
+        set_expiration_if_necessary
         true
       end
     end
@@ -59,7 +59,6 @@ def delete!
     def lock(timeout = 0)
       exists_or_create!
       release_stale_locks! if check_staleness?
-
       token_pair = @redis.blpop(available_key, timeout)
       return false if token_pair.nil?
 
@@ -157,8 +156,6 @@ def simple_mutex(key_name, expires = nil)
     end
 
     def create!
-      @redis.expire(exists_key, 10)
-
       @redis.multi do
         @redis.del(grabbed_key)
         @redis.del(available_key)
@@ -167,7 +164,6 @@ def create!
         end
         @redis.set(version_key, API_VERSION)
         @redis.persist(exists_key)
-
         set_expiration_if_necessary
       end
     end

spec/semaphore_spec.rb

@@ -149,23 +149,25 @@
       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
+    it "does not leave a key without expiration if expiration given" do
       queue = Queue.new
-      threads = 2.times.collect do
+      threads = 2.times.map do
         Thread.new do
-          Redis::Semaphore.new(:my_semaphore, :redis => @redis, :expiration => 3).lock(5) do
+          opts = { redis: @redis, expiration: 3 }
+          Redis::Semaphore.new(:my_semaphore, opts).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
+      @redis2 = Redis.new db: 15
+      threads.each(&:kill) # ensure signal step fails
+      exist_key = @redis2.ttl("SEMAPHORE:my_semaphore:EXISTS").to_s
+      expect(exist_key).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
+        opts = { redis: @redis2, expiration: 3 }
+        Redis::Semaphore.new(:my_semaphore, opts).lock(5) do
           queue << "work"
         end
       end