Lock extension via lock method allows extending expired locks

[jonp]

If your code allows the lock to expire and then you call lock to extend the life of the lock, it will allow it. It would be really handy to have it fail if the lock was allowed to expire. This has been added to the node and .NET versions of the algorithm, so there's some precedent.

[jonp]

I just submitted pull request #37 to add extend functionality. Let me know if you need anything else!

[leandromoreira]

Thanks @jonp isn't this already done at #32 ?

[jonp]

Well, not quite. It lets you extend a lock that has been expired. Consider this script:

require 'redlock'
require 'redis'

shared_data = 1
key = "some key"
lock = :start

redis = Redis.new
mgr = Redlock::Client.new([redis])

t1 = Thread.new do
  mgr.lock(key, 2) do |locked|
    shared_data += 1

    # simulate expiration and eviction of the lock key
    redis.del(key)
    lock= :one
    while lock != :two; sleep(0.01); end

    # now try to extend the expired lock - it works as if we always had the lock
    mgr.lock(key, 2, extend: locked)

    # the data shouldn't be what we expected
    raise "Hey! Who's been messing with my data?" if shared_data != 2
  end
end

t2 = Thread.new do
  while lock != :one; sleep(0.01); end

  # grab the lock while thread 1 has let it expire and mess up the data it thought it owned
  mgr.lock(key, 2) do |locked|
    shared_data = 100
  end
  lock = :two
end

[t1, t2].each(&:join)

Note that the line mgr.lock(key, 2, extend: locked) successfully acquires the lock even though the lock is already expired, which is not necessarily wrong; just surprising. My addition of extend is based on my assumption that that behavior was desired and that a separate call was needed rather than changing the semantics of the lock call.

[leandromoreira]

you got a point

[leandromoreira]

#37

[jonp]

Hi Leandro,

Thanks for taking my pull request for lock extensions!
I’m wondering whether you’d be interested in a pull request to redlock-rb that would make it ruby-1.9-compatible.

• Jon Phillips

[leandromoreira]

This is something we already discussed @maltoe but maybe for Ruby-1-9 is reasonable. What do you think @maltoe ?

Ruby-1-9 is 6 years old too!

[jonp]

Yes, 1.9.3 is quite old, but lots of people are stuck on it for various reasons. I found that this gem's incompatibility with 1.9.3 is just a couple of uses of keyword parameters and a %i...about 5 minutes worth of fiddling to make it work.

[leandromoreira]

I'm okay @jonp :)

[seamusabshere]

@jonp @leandromoreira i contributed the lock functionality and this is an error on my part:

My addition of extend is based on my assumption that that behavior was desired and that a separate call was needed rather than changing the semantics of the lock call.

i would prefer that the original extend functionality be fixed instead of adding another complete, but slightly different implementation

[jonp]

@seamusabshere that works for me, although there may be some users that now expect the existing semantics.

[leandromoreira]

Great idea! Cool Thank you so much @seamusabshere 🍻