From 5332d2a5142f371c2fd8b247df5afe9c75874a21 Mon Sep 17 00:00:00 2001 From: EddieChoCho Date: Sat, 12 Aug 2023 21:47:28 +0800 Subject: [PATCH] GH-8699: Enhancing unlock() Method of RedisLock with Atomic Redis Operation --- .../redis/util/RedisLockRegistry.java | 98 +++++++++++++------ 1 file changed, 67 insertions(+), 31 deletions(-) diff --git a/spring-integration-redis/src/main/java/org/springframework/integration/redis/util/RedisLockRegistry.java b/spring-integration-redis/src/main/java/org/springframework/integration/redis/util/RedisLockRegistry.java index 828c18701d5..1261fdb9eff 100644 --- a/spring-integration-redis/src/main/java/org/springframework/integration/redis/util/RedisLockRegistry.java +++ b/spring-integration-redis/src/main/java/org/springframework/integration/redis/util/RedisLockRegistry.java @@ -343,12 +343,12 @@ public long getLockedAt() { /** * Unlock the lock using the unlink method in redis. */ - protected abstract void removeLockKeyInnerUnlink(); + protected abstract boolean removeLockKeyInnerUnlink(); /** * Unlock the lock using the delete method in redis. */ - protected abstract void removeLockKeyInnerDelete(); + protected abstract boolean removeLockKeyInnerDelete(); @Override public final void lock() { @@ -454,11 +454,6 @@ public final void unlock() { return; } try { - if (!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."); - } - if (Thread.currentThread().isInterrupted()) { RedisLockRegistry.this.executor.execute(this::removeLockKey); } @@ -480,8 +475,9 @@ public final void unlock() { private void removeLockKey() { if (RedisLockRegistry.this.unlinkAvailable) { + Boolean unlinkResult = null; try { - removeLockKeyInnerUnlink(); + unlinkResult = removeLockKeyInnerUnlink(); return; } catch (Exception ex) { @@ -494,9 +490,17 @@ private void removeLockKey() { LOGGER.warn("The UNLINK command has failed (not supported on the Redis server?); " + "falling back to the regular DELETE command: " + ex.getMessage()); } + } finally { + if (Boolean.FALSE.equals(unlinkResult)) { + throw new IllegalStateException("Lock was released in the store due to expiration. " + + "The integrity of data protected by this lock may have been compromised."); + } } } - removeLockKeyInnerDelete(); + if (!removeLockKeyInnerDelete()) { + throw new IllegalStateException("Lock was released in the store due to expiration. " + + "The integrity of data protected by this lock may have been compromised."); + } } @Override @@ -559,19 +563,23 @@ private RedisLockRegistry getOuterType() { private final class RedisPubSubLock extends RedisLock { - private static final String UNLINK_UNLOCK_SCRIPT = - "if (redis.call('unlink', KEYS[1]) == 1) then " + - "redis.call('publish', ARGV[1], KEYS[1]) " + - "return true " + - "end " + - "return false"; + private static final String UNLINK_UNLOCK_SCRIPT = """ + local lockClientId = redis.call('GET', KEYS[1]) + if (lockClientId == ARGV[1] and redis.call('UNLINK', KEYS[1]) == 1) then + redis.call('PUBLISH', ARGV[2], KEYS[1]) + return true + end + return false + """; - private static final String DELETE_UNLOCK_SCRIPT = - "if (redis.call('del', KEYS[1]) == 1) then " + - "redis.call('publish', ARGV[1], KEYS[1]) " + - "return true " + - "end " + - "return false"; + private static final String DELETE_UNLOCK_SCRIPT = """ + local lockClientId = redis.call('GET', KEYS[1]) + if (lockClientId == ARGV[1] and redis.call('DEL', KEYS[1]) == 1) then + redis.call('PUBLISH', ARGV[2], KEYS[1]) + return true + end + return false + """; private static final RedisScript UNLINK_UNLOCK_REDIS_SCRIPT = new DefaultRedisScript<>(UNLINK_UNLOCK_SCRIPT, Boolean.class); @@ -589,17 +597,17 @@ protected boolean tryRedisLockInner(long time) throws ExecutionException, Interr } @Override - protected void removeLockKeyInnerUnlink() { - RedisLockRegistry.this.redisTemplate.execute( + protected boolean removeLockKeyInnerUnlink() { + return Boolean.TRUE.equals(RedisLockRegistry.this.redisTemplate.execute( UNLINK_UNLOCK_REDIS_SCRIPT, Collections.singletonList(this.lockKey), - RedisLockRegistry.this.unLockChannelKey); + RedisLockRegistry.this.clientId, RedisLockRegistry.this.unLockChannelKey)); } @Override - protected void removeLockKeyInnerDelete() { - RedisLockRegistry.this.redisTemplate.execute( + protected boolean removeLockKeyInnerDelete() { + return Boolean.TRUE.equals(RedisLockRegistry.this.redisTemplate.execute( DELETE_UNLOCK_REDIS_SCRIPT, Collections.singletonList(this.lockKey), - RedisLockRegistry.this.unLockChannelKey); + RedisLockRegistry.this.clientId, RedisLockRegistry.this.unLockChannelKey)); } @@ -694,6 +702,30 @@ private void unlockNotify(String lockKey) { private final class RedisSpinLock extends RedisLock { + private static final String UNLINK_UNLOCK_SCRIPT = """ + local lockClientId = redis.call('GET', KEYS[1]) + if lockClientId == ARGV[1] then + redis.call('UNLINK', KEYS[1]) + return true + end + return false + """; + + private static final String DELETE_UNLOCK_SCRIPT = """ + local lockClientId = redis.call('GET', KEYS[1]) + if lockClientId == ARGV[1] then + redis.call('DEL', KEYS[1]) + return true + end + return false + """; + + private static final RedisScript + UNLINK_UNLOCK_REDIS_SCRIPT = new DefaultRedisScript<>(UNLINK_UNLOCK_SCRIPT, Boolean.class); + + private static final RedisScript + DELETE_UNLOCK_REDIS_SCRIPT = new DefaultRedisScript<>(DELETE_UNLOCK_SCRIPT, Boolean.class); + private RedisSpinLock(String path) { super(path); } @@ -718,13 +750,17 @@ protected boolean tryRedisLockInner(long time) throws InterruptedException { } @Override - protected void removeLockKeyInnerUnlink() { - RedisLockRegistry.this.redisTemplate.unlink(this.lockKey); + protected boolean removeLockKeyInnerUnlink() { + return Boolean.TRUE.equals(RedisLockRegistry.this.redisTemplate.execute( + UNLINK_UNLOCK_REDIS_SCRIPT, Collections.singletonList(this.lockKey), + RedisLockRegistry.this.clientId)); } @Override - protected void removeLockKeyInnerDelete() { - RedisLockRegistry.this.redisTemplate.delete(this.lockKey); + protected boolean removeLockKeyInnerDelete() { + return Boolean.TRUE.equals(RedisLockRegistry.this.redisTemplate.execute( + DELETE_UNLOCK_REDIS_SCRIPT, Collections.singletonList(this.lockKey), + RedisLockRegistry.this.clientId)); } }