Skip to content

Commit 2e62707

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`** # Conflicts: # spring-integration-redis/src/main/java/org/springframework/integration/redis/util/RedisLockRegistry.java
1 parent b81ab75 commit 2e62707

File tree

1 file changed

+73
-35
lines changed

1 file changed

+73
-35
lines changed

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

Lines changed: 73 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@
8282
* @author Vedran Pavic
8383
* @author Unseok Kim
8484
* @author Anton Gabov
85+
* @author Eddie Cho
8586
*
8687
* @since 4.0
8788
*
@@ -95,7 +96,7 @@ public final class RedisLockRegistry implements ExpirableLockRegistry, Disposabl
9596
private static final int DEFAULT_CAPACITY = 100_000;
9697

9798
private final Map<String, RedisLock> locks =
98-
new LinkedHashMap<String, RedisLock>(16, 0.75F, true) {
99+
new LinkedHashMap<>(16, 0.75F, true) {
99100

100101
@Override
101102
protected boolean removeEldestEntry(Entry<String, RedisLock> eldest) {
@@ -331,12 +332,12 @@ public long getLockedAt() {
331332
/**
332333
* Unlock the lock using the unlink method in redis.
333334
*/
334-
protected abstract void removeLockKeyInnerUnlink();
335+
protected abstract boolean removeLockKeyInnerUnlink();
335336

336337
/**
337338
* Unlock the lock using the delete method in redis.
338339
*/
339-
protected abstract void removeLockKeyInnerDelete();
340+
protected abstract boolean removeLockKeyInnerDelete();
340341

341342
@Override
342343
public final void lock() {
@@ -442,11 +443,6 @@ public final void unlock() {
442443
return;
443444
}
444445
try {
445-
if (!isAcquiredInThisProcess()) {
446-
throw new IllegalStateException("Lock was released in the store due to expiration. " +
447-
"The integrity of data protected by this lock may have been compromised.");
448-
}
449-
450446
if (Thread.currentThread().isInterrupted()) {
451447
RedisLockRegistry.this.executor.execute(this::removeLockKey);
452448
}
@@ -469,7 +465,11 @@ public final void unlock() {
469465
private void removeLockKey() {
470466
if (RedisLockRegistry.this.unlinkAvailable) {
471467
try {
472-
removeLockKeyInnerUnlink();
468+
boolean unlinkResult = removeLockKeyInnerUnlink();
469+
if (!unlinkResult) {
470+
throw new IllegalStateException("Lock was released in the store due to expiration. " +
471+
"The integrity of data protected by this lock may have been compromised.");
472+
}
473473
return;
474474
}
475475
catch (Exception ex) {
@@ -484,7 +484,10 @@ private void removeLockKey() {
484484
}
485485
}
486486
}
487-
removeLockKeyInnerDelete();
487+
if (!removeLockKeyInnerDelete()) {
488+
throw new IllegalStateException("Lock was released in the store due to expiration. " +
489+
"The integrity of data protected by this lock may have been compromised.");
490+
}
488491
}
489492

490493
@Override
@@ -547,19 +550,23 @@ private RedisLockRegistry getOuterType() {
547550

548551
private final class RedisPubSubLock extends RedisLock {
549552

550-
private static final String UNLINK_UNLOCK_SCRIPT =
551-
"if (redis.call('unlink', KEYS[1]) == 1) then " +
552-
"redis.call('publish', ARGV[1], KEYS[1]) " +
553-
"return true " +
554-
"end " +
555-
"return false";
556-
557-
private static final String DELETE_UNLOCK_SCRIPT =
558-
"if (redis.call('del', KEYS[1]) == 1) then " +
559-
"redis.call('publish', ARGV[1], KEYS[1]) " +
560-
"return true " +
561-
"end " +
562-
"return false";
553+
private static final String UNLINK_UNLOCK_SCRIPT = """
554+
local lockClientId = redis.call('GET', KEYS[1])
555+
if (lockClientId == ARGV[1] and redis.call('UNLINK', KEYS[1]) == 1) then
556+
redis.call('PUBLISH', ARGV[2], KEYS[1])
557+
return true
558+
end
559+
return false
560+
""";
561+
562+
private static final String DELETE_UNLOCK_SCRIPT = """
563+
local lockClientId = redis.call('GET', KEYS[1])
564+
if (lockClientId == ARGV[1] and redis.call('DEL', KEYS[1]) == 1) then
565+
redis.call('PUBLISH', ARGV[2], KEYS[1])
566+
return true
567+
end
568+
return false
569+
""";
563570

564571
private static final RedisScript<Boolean>
565572
UNLINK_UNLOCK_REDIS_SCRIPT = new DefaultRedisScript<>(UNLINK_UNLOCK_SCRIPT, Boolean.class);
@@ -577,18 +584,19 @@ protected boolean tryRedisLockInner(long time) throws ExecutionException, Interr
577584
}
578585

579586
@Override
580-
protected void removeLockKeyInnerUnlink() {
581-
RedisLockRegistry.this.redisTemplate.execute(
582-
UNLINK_UNLOCK_REDIS_SCRIPT, Collections.singletonList(this.lockKey),
583-
RedisLockRegistry.this.unLockChannelKey);
587+
protected boolean removeLockKeyInnerUnlink() {
588+
return removeLockKeyWithScript(UNLINK_UNLOCK_REDIS_SCRIPT);
584589
}
585590

586591
@Override
587-
protected void removeLockKeyInnerDelete() {
588-
RedisLockRegistry.this.redisTemplate.execute(
589-
DELETE_UNLOCK_REDIS_SCRIPT, Collections.singletonList(this.lockKey),
590-
RedisLockRegistry.this.unLockChannelKey);
592+
protected boolean removeLockKeyInnerDelete() {
593+
return removeLockKeyWithScript(DELETE_UNLOCK_REDIS_SCRIPT);
594+
}
591595

596+
private boolean removeLockKeyWithScript(RedisScript<Boolean> redisScript) {
597+
return Boolean.TRUE.equals(RedisLockRegistry.this.redisTemplate.execute(
598+
redisScript, Collections.singletonList(this.lockKey),
599+
RedisLockRegistry.this.clientId, RedisLockRegistry.this.unLockChannelKey));
592600
}
593601

594602
private boolean subscribeLock(long time) throws ExecutionException, InterruptedException {
@@ -678,6 +686,30 @@ private void unlockNotify(String lockKey) {
678686

679687
private final class RedisSpinLock extends RedisLock {
680688

689+
private static final String UNLINK_UNLOCK_SCRIPT = """
690+
local lockClientId = redis.call('GET', KEYS[1])
691+
if lockClientId == ARGV[1] then
692+
redis.call('UNLINK', KEYS[1])
693+
return true
694+
end
695+
return false
696+
""";
697+
698+
private static final String DELETE_UNLOCK_SCRIPT = """
699+
local lockClientId = redis.call('GET', KEYS[1])
700+
if lockClientId == ARGV[1] then
701+
redis.call('DEL', KEYS[1])
702+
return true
703+
end
704+
return false
705+
""";
706+
707+
private static final RedisScript<Boolean>
708+
UNLINK_UNLOCK_REDIS_SCRIPT = new DefaultRedisScript<>(UNLINK_UNLOCK_SCRIPT, Boolean.class);
709+
710+
private static final RedisScript<Boolean>
711+
DELETE_UNLOCK_REDIS_SCRIPT = new DefaultRedisScript<>(DELETE_UNLOCK_SCRIPT, Boolean.class);
712+
681713
private RedisSpinLock(String path) {
682714
super(path);
683715
}
@@ -702,13 +734,19 @@ protected boolean tryRedisLockInner(long time) throws InterruptedException {
702734
}
703735

704736
@Override
705-
protected void removeLockKeyInnerUnlink() {
706-
RedisLockRegistry.this.redisTemplate.unlink(this.lockKey);
737+
protected boolean removeLockKeyInnerUnlink() {
738+
return removeLockKeyWithScript(UNLINK_UNLOCK_REDIS_SCRIPT);
707739
}
708740

709741
@Override
710-
protected void removeLockKeyInnerDelete() {
711-
RedisLockRegistry.this.redisTemplate.delete(this.lockKey);
742+
protected boolean removeLockKeyInnerDelete() {
743+
return removeLockKeyWithScript(DELETE_UNLOCK_REDIS_SCRIPT);
744+
}
745+
746+
private boolean removeLockKeyWithScript(RedisScript<Boolean> redisScript) {
747+
return Boolean.TRUE.equals(RedisLockRegistry.this.redisTemplate.execute(
748+
redisScript, Collections.singletonList(this.lockKey),
749+
RedisLockRegistry.this.clientId));
712750
}
713751

714752
}

0 commit comments

Comments
 (0)