Skip to content

Commit 0450a32

Browse files
EddieChoChoartembilan
authored andcommitted
GH-8699: Atomic Redis script for unlock()
Expected Behavior Using a single Lua script to verify ownership of the lock and remove it. Current Behavior `unlock()` method of `RedisLock` uses two separate Redis operations: * `isAcquiredInThisProcess()`` method executes a `GET` operation to verify if the lock is owned by the process. * `removeLockKey()`` method executes `UNLINK/DEL` operation to remove the lock. * The `removeLockKeyInnerUnlink()`, and `removeLockKeyInnerDelete()` methods will execute a script both verify ownership of the lock and remove it. **Cherry-pick to `6.1.x` & `6.0.x`**
1 parent 92fb0d3 commit 0450a32

File tree

1 file changed

+72
-34
lines changed

1 file changed

+72
-34
lines changed

spring-integration-redis/src/main/java/org/springframework/integration/redis/util/RedisLockRegistry.java

+72-34
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@
8383
* @author Unseok Kim
8484
* @author Anton Gabov
8585
* @author Christian Tzolov
86+
* @author Eddie Cho
8687
*
8788
* @since 4.0
8889
*
@@ -98,7 +99,7 @@ public final class RedisLockRegistry implements ExpirableLockRegistry, Disposabl
9899
private final Lock lock = new ReentrantLock();
99100

100101
private final Map<String, RedisLock> locks =
101-
new LinkedHashMap<String, RedisLock>(16, 0.75F, true) {
102+
new LinkedHashMap<>(16, 0.75F, true) {
102103

103104
@Override
104105
protected boolean removeEldestEntry(Entry<String, RedisLock> eldest) {
@@ -343,12 +344,12 @@ public long getLockedAt() {
343344
/**
344345
* Unlock the lock using the unlink method in redis.
345346
*/
346-
protected abstract void removeLockKeyInnerUnlink();
347+
protected abstract boolean removeLockKeyInnerUnlink();
347348

348349
/**
349350
* Unlock the lock using the delete method in redis.
350351
*/
351-
protected abstract void removeLockKeyInnerDelete();
352+
protected abstract boolean removeLockKeyInnerDelete();
352353

353354
@Override
354355
public final void lock() {
@@ -454,11 +455,6 @@ public final void unlock() {
454455
return;
455456
}
456457
try {
457-
if (!isAcquiredInThisProcess()) {
458-
throw new IllegalStateException("Lock was released in the store due to expiration. " +
459-
"The integrity of data protected by this lock may have been compromised.");
460-
}
461-
462458
if (Thread.currentThread().isInterrupted()) {
463459
RedisLockRegistry.this.executor.execute(this::removeLockKey);
464460
}
@@ -481,7 +477,11 @@ public final void unlock() {
481477
private void removeLockKey() {
482478
if (RedisLockRegistry.this.unlinkAvailable) {
483479
try {
484-
removeLockKeyInnerUnlink();
480+
boolean unlinkResult = removeLockKeyInnerUnlink();
481+
if (!unlinkResult) {
482+
throw new IllegalStateException("Lock was released in the store due to expiration. " +
483+
"The integrity of data protected by this lock may have been compromised.");
484+
}
485485
return;
486486
}
487487
catch (Exception ex) {
@@ -496,7 +496,10 @@ private void removeLockKey() {
496496
}
497497
}
498498
}
499-
removeLockKeyInnerDelete();
499+
if (!removeLockKeyInnerDelete()) {
500+
throw new IllegalStateException("Lock was released in the store due to expiration. " +
501+
"The integrity of data protected by this lock may have been compromised.");
502+
}
500503
}
501504

502505
@Override
@@ -559,19 +562,23 @@ private RedisLockRegistry getOuterType() {
559562

560563
private final class RedisPubSubLock extends RedisLock {
561564

562-
private static final String UNLINK_UNLOCK_SCRIPT =
563-
"if (redis.call('unlink', KEYS[1]) == 1) then " +
564-
"redis.call('publish', ARGV[1], KEYS[1]) " +
565-
"return true " +
566-
"end " +
567-
"return false";
565+
private static final String UNLINK_UNLOCK_SCRIPT = """
566+
local lockClientId = redis.call('GET', KEYS[1])
567+
if (lockClientId == ARGV[1] and redis.call('UNLINK', KEYS[1]) == 1) then
568+
redis.call('PUBLISH', ARGV[2], KEYS[1])
569+
return true
570+
end
571+
return false
572+
""";
568573

569-
private static final String DELETE_UNLOCK_SCRIPT =
570-
"if (redis.call('del', KEYS[1]) == 1) then " +
571-
"redis.call('publish', ARGV[1], KEYS[1]) " +
572-
"return true " +
573-
"end " +
574-
"return false";
574+
private static final String DELETE_UNLOCK_SCRIPT = """
575+
local lockClientId = redis.call('GET', KEYS[1])
576+
if (lockClientId == ARGV[1] and redis.call('DEL', KEYS[1]) == 1) then
577+
redis.call('PUBLISH', ARGV[2], KEYS[1])
578+
return true
579+
end
580+
return false
581+
""";
575582

576583
private static final RedisScript<Boolean>
577584
UNLINK_UNLOCK_REDIS_SCRIPT = new DefaultRedisScript<>(UNLINK_UNLOCK_SCRIPT, Boolean.class);
@@ -589,18 +596,19 @@ protected boolean tryRedisLockInner(long time) throws ExecutionException, Interr
589596
}
590597

591598
@Override
592-
protected void removeLockKeyInnerUnlink() {
593-
RedisLockRegistry.this.redisTemplate.execute(
594-
UNLINK_UNLOCK_REDIS_SCRIPT, Collections.singletonList(this.lockKey),
595-
RedisLockRegistry.this.unLockChannelKey);
599+
protected boolean removeLockKeyInnerUnlink() {
600+
return removeLockKeyWithScript(UNLINK_UNLOCK_REDIS_SCRIPT);
596601
}
597602

598603
@Override
599-
protected void removeLockKeyInnerDelete() {
600-
RedisLockRegistry.this.redisTemplate.execute(
601-
DELETE_UNLOCK_REDIS_SCRIPT, Collections.singletonList(this.lockKey),
602-
RedisLockRegistry.this.unLockChannelKey);
604+
protected boolean removeLockKeyInnerDelete() {
605+
return removeLockKeyWithScript(DELETE_UNLOCK_REDIS_SCRIPT);
606+
}
603607

608+
private boolean removeLockKeyWithScript(RedisScript<Boolean> redisScript) {
609+
return Boolean.TRUE.equals(RedisLockRegistry.this.redisTemplate.execute(
610+
redisScript, Collections.singletonList(this.lockKey),
611+
RedisLockRegistry.this.clientId, RedisLockRegistry.this.unLockChannelKey));
604612
}
605613

606614
private boolean subscribeLock(long time) throws ExecutionException, InterruptedException {
@@ -694,6 +702,30 @@ private void unlockNotify(String lockKey) {
694702

695703
private final class RedisSpinLock extends RedisLock {
696704

705+
private static final String UNLINK_UNLOCK_SCRIPT = """
706+
local lockClientId = redis.call('GET', KEYS[1])
707+
if lockClientId == ARGV[1] then
708+
redis.call('UNLINK', KEYS[1])
709+
return true
710+
end
711+
return false
712+
""";
713+
714+
private static final String DELETE_UNLOCK_SCRIPT = """
715+
local lockClientId = redis.call('GET', KEYS[1])
716+
if lockClientId == ARGV[1] then
717+
redis.call('DEL', KEYS[1])
718+
return true
719+
end
720+
return false
721+
""";
722+
723+
private static final RedisScript<Boolean>
724+
UNLINK_UNLOCK_REDIS_SCRIPT = new DefaultRedisScript<>(UNLINK_UNLOCK_SCRIPT, Boolean.class);
725+
726+
private static final RedisScript<Boolean>
727+
DELETE_UNLOCK_REDIS_SCRIPT = new DefaultRedisScript<>(DELETE_UNLOCK_SCRIPT, Boolean.class);
728+
697729
private RedisSpinLock(String path) {
698730
super(path);
699731
}
@@ -718,13 +750,19 @@ protected boolean tryRedisLockInner(long time) throws InterruptedException {
718750
}
719751

720752
@Override
721-
protected void removeLockKeyInnerUnlink() {
722-
RedisLockRegistry.this.redisTemplate.unlink(this.lockKey);
753+
protected boolean removeLockKeyInnerUnlink() {
754+
return removeLockKeyWithScript(UNLINK_UNLOCK_REDIS_SCRIPT);
723755
}
724756

725757
@Override
726-
protected void removeLockKeyInnerDelete() {
727-
RedisLockRegistry.this.redisTemplate.delete(this.lockKey);
758+
protected boolean removeLockKeyInnerDelete() {
759+
return removeLockKeyWithScript(DELETE_UNLOCK_REDIS_SCRIPT);
760+
}
761+
762+
private boolean removeLockKeyWithScript(RedisScript<Boolean> redisScript) {
763+
return Boolean.TRUE.equals(RedisLockRegistry.this.redisTemplate.execute(
764+
redisScript, Collections.singletonList(this.lockKey),
765+
RedisLockRegistry.this.clientId));
728766
}
729767

730768
}

0 commit comments

Comments
 (0)