Is this a bug when lock is timeout it throws an excption in spring-integration-redis?

[Farwell-Liu]

I use spring-integration-redis for distibution lock and I found that if the lock key is expired, it will throw an exception when I unlock the lock. here is my code:

RedisLockRegistry registry = new RedisLockRegistry(getConnectionFactory(), this.registryKey);
            Lock lock = registry.obtain("foo");
            lock.lock();
            try {
                //do something that may be expired
            }
           finally {
               lock.unlock();//it throws an exception if the redis key is expired
           }
}

I read the source code of 'unlock' method in RedisLockRegistry class I found that:

if (!this.isAcquiredInThisProcess()) {
                        throw new IllegalStateException("Lock was released in the store due to expiration. The integrity of data protected by this lock may have been compromised.");
                    }

It seems that it is not a bug. I think it mislead the developers likely, at least it mislead me. so, my question is that why not just ignore case that the redis key is expired. Isn't it more friendly to developers. And then I needn't to embeded a try catch block in finally block above.

My English is not good enough probably, Did I describe my question clearly?
Could you please explain that for me? thank you very much!

[garyrussell]

The problem is that if a lock expires while the application has it locked, it means that the data protected by the lock is compromised - some other process could have changed it.

The exception is to tell the application that it has happened so that it can take some action to verify the data is ok.

If we just exit silently, the application would not know that the data has been compromised.

[artembilan]

We have a ticket to react to the event in case of key expiration: https://jira.spring.io/browse/INT-4286.

But that doesn't mean that we are going to change the logic in this unlock() implementation.

We'll think about something internally, but I'm not sure that we can interfere somehow into the locking thread...

[Farwell-Liu]

Thank you garyrussell and artembilan! I think that I have undertood the reason.But I am thinking that should you provide method like this?

Lock lock = registry.obtain("foo");
lock.lock();
try {
    //do something that may be expired
}
finally {
   ResuntEnum result= lock.unlock();//normally it returns Success,if expired then it returns KeyExpired
   if(result==ResultEnum.KeyExpried){
      //do something to deal with compromised data
   }
}

artembilan, did you mean that RedisLockRegistry provide method overload:
lock.unlock(new KeyExpirationEventMessageListener(){....});//to implement a interface
Or:
lock.unlock(v->{....});//lambda expression to deal with key expired

In my opinion all the methods above is feasible, and it avoids to use embeded try catch block.
Would you consider implementing this function in future?

[artembilan]

Everything is just based on the java.util.concurrent.locks.Lock contract, so we definitely are not going to interfere that API.

So, no one your suggestion is feasible. Sorry for that...

I'm not sure what we are going to do there: need to jump into that and think...

May be next week I can switch to that ticket.

But for now, I think, this issue is covered with our discussion and looks like there is nothing to do, so let's consider to close it already!

Thanks for your time!

[Farwell-Liu]

I got it, Thank you very much!