From ee3d4c2e37baf52a169b491e8d929cf6bcc6bca6 Mon Sep 17 00:00:00 2001 From: seakider Date: Wed, 18 Dec 2024 19:27:59 +0800 Subject: [PATCH 1/4] Fixed - RPermitExpirableSemaphore.release(java.util.List) shouldn't release permits if one of them doesn't exist #6343 Signed-off-by: xuxiaolei --- .../RedissonPermitExpirableSemaphore.java | 16 ++++++++---- .../RedissonPermitExpirableSemaphoreTest.java | 26 +++++++++++++++++++ 2 files changed, 37 insertions(+), 5 deletions(-) diff --git a/redisson/src/main/java/org/redisson/RedissonPermitExpirableSemaphore.java b/redisson/src/main/java/org/redisson/RedissonPermitExpirableSemaphore.java index a1cf3d6a8..39203ac47 100644 --- a/redisson/src/main/java/org/redisson/RedissonPermitExpirableSemaphore.java +++ b/redisson/src/main/java/org/redisson/RedissonPermitExpirableSemaphore.java @@ -657,6 +657,12 @@ public class RedissonPermitExpirableSemaphore extends RedissonExpirable implemen } return commandExecutor.syncedEvalWithRetry(getRawName(), LongCodec.INSTANCE, RedisCommands.EVAL_INTEGER, + "for i = 4, #ARGV, 1 do " + + "local expire = redis.call('zscore', KEYS[3], ARGV[i]);" + + "if expire== false or tonumber(expire) <= tonumber(ARGV[2]) then " + + "return 0;" + + "end; " + + "end; " + "local expiredIds = redis.call('zrangebyscore', KEYS[3], 0, ARGV[2], 'limit', 0, -1); " + "if #expiredIds > 0 then " + "redis.call('zrem', KEYS[3], unpack(expiredIds)); " + @@ -670,9 +676,6 @@ public class RedissonPermitExpirableSemaphore extends RedissonExpirable implemen "table.insert(keys, ARGV[i]); " + "end; " + "local removed = redis.call('zrem', KEYS[3], unpack(keys)); " + - "if tonumber(removed) == 0 then " + - "return 0;" + - "end; " + "redis.call('incrby', KEYS[1], removed); " + "redis.call(ARGV[3], KEYS[2], removed); " + "return removed;", @@ -733,7 +736,10 @@ public class RedissonPermitExpirableSemaphore extends RedissonExpirable implemen throw new CompletionException(e); } - if (res == permitsIds.size()) { +// if (res == permitsIds.size()) { +// return null; +// } + if (res != 0) { return null; } throw new CompletionException(new IllegalArgumentException("Permits with ids " + permitsIds + " have already been released or don't exist")); @@ -931,7 +937,7 @@ public class RedissonPermitExpirableSemaphore extends RedissonExpirable implemen if (e != null) { throw new CompletionException(e); } - + if (res == 0) { throw new CompletionException(new IllegalArgumentException("Permit with id " + permitId + " has already been released or doesn't exist")); } diff --git a/redisson/src/test/java/org/redisson/RedissonPermitExpirableSemaphoreTest.java b/redisson/src/test/java/org/redisson/RedissonPermitExpirableSemaphoreTest.java index 0b8fc1495..e0f08c9a5 100644 --- a/redisson/src/test/java/org/redisson/RedissonPermitExpirableSemaphoreTest.java +++ b/redisson/src/test/java/org/redisson/RedissonPermitExpirableSemaphoreTest.java @@ -476,6 +476,7 @@ public class RedissonPermitExpirableSemaphoreTest extends BaseConcurrentTest { assertThat(semaphore.availablePermits()).isEqualTo(10); Assertions.assertThrows(RedisException.class, () -> semaphore.release(permitsIds)); + } @Test @@ -553,6 +554,31 @@ public class RedissonPermitExpirableSemaphoreTest extends BaseConcurrentTest { assertThat(released).isEqualTo(0); assertThat(s.availablePermits()).isEqualTo(10); } + + @Test + public void testReleaseManyWithout() throws InterruptedException { + RPermitExpirableSemaphore s = redisson.getPermitExpirableSemaphore("test"); + s.trySetPermits(10); + List timedPermitsIds = s.acquire(2,100, TimeUnit.MILLISECONDS); + List permitsIds = s.tryAcquire(8); + List permitsIdsFirstPart = permitsIds.subList(0, 2); + + int released = s.tryRelease(permitsIdsFirstPart); + assertThat(released).isEqualTo(2); + assertThat(s.availablePermits()).isEqualTo(2); + + Thread.sleep(100); + + Assertions.assertThrows(RedisException.class, () -> s.release(timedPermitsIds)); + assertThat(s.availablePermits()).isEqualTo(4); + + List permitsIdsThirdPart = permitsIds.subList(4, 6); + permitsIdsThirdPart.addAll(permitsIdsFirstPart); + Assertions.assertThrows(RedisException.class, () -> s.release(permitsIdsThirdPart)); + assertThat(s.availablePermits()).isEqualTo(4); + + + } @Test public void testGetLeaseTime() throws InterruptedException { From 4d6ed7d72b4825f7d96932ff1a516257428b4b3a Mon Sep 17 00:00:00 2001 From: seakider Date: Wed, 18 Dec 2024 20:57:54 +0800 Subject: [PATCH 2/4] Fixed - RPermitExpirableSemaphore.release(java.util.List) shouldn't release permits if one of them doesn't exist #6343 Signed-off-by: xuxiaolei --- .../redisson/RedissonPermitExpirableSemaphore.java | 6 ++---- .../RedissonPermitExpirableSemaphoreTest.java | 13 ++++++------- 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/redisson/src/main/java/org/redisson/RedissonPermitExpirableSemaphore.java b/redisson/src/main/java/org/redisson/RedissonPermitExpirableSemaphore.java index 39203ac47..7d25c479c 100644 --- a/redisson/src/main/java/org/redisson/RedissonPermitExpirableSemaphore.java +++ b/redisson/src/main/java/org/redisson/RedissonPermitExpirableSemaphore.java @@ -736,12 +736,10 @@ public class RedissonPermitExpirableSemaphore extends RedissonExpirable implemen throw new CompletionException(e); } -// if (res == permitsIds.size()) { -// return null; -// } - if (res != 0) { + if (res == permitsIds.size()) { return null; } + throw new CompletionException(new IllegalArgumentException("Permits with ids " + permitsIds + " have already been released or don't exist")); }); return new CompletableFutureWrapper<>(f); diff --git a/redisson/src/test/java/org/redisson/RedissonPermitExpirableSemaphoreTest.java b/redisson/src/test/java/org/redisson/RedissonPermitExpirableSemaphoreTest.java index e0f08c9a5..18ec8e6de 100644 --- a/redisson/src/test/java/org/redisson/RedissonPermitExpirableSemaphoreTest.java +++ b/redisson/src/test/java/org/redisson/RedissonPermitExpirableSemaphoreTest.java @@ -556,11 +556,12 @@ public class RedissonPermitExpirableSemaphoreTest extends BaseConcurrentTest { } @Test - public void testReleaseManyWithout() throws InterruptedException { + public void testReleaseManyNotExistOrExpired() throws InterruptedException { RPermitExpirableSemaphore s = redisson.getPermitExpirableSemaphore("test"); s.trySetPermits(10); - List timedPermitsIds = s.acquire(2,100, TimeUnit.MILLISECONDS); + List timedPermitsIds = s.acquire(2, 100, TimeUnit.MILLISECONDS); List permitsIds = s.tryAcquire(8); + List permitsIdsFirstPart = permitsIds.subList(0, 2); int released = s.tryRelease(permitsIdsFirstPart); @@ -572,12 +573,10 @@ public class RedissonPermitExpirableSemaphoreTest extends BaseConcurrentTest { Assertions.assertThrows(RedisException.class, () -> s.release(timedPermitsIds)); assertThat(s.availablePermits()).isEqualTo(4); - List permitsIdsThirdPart = permitsIds.subList(4, 6); - permitsIdsThirdPart.addAll(permitsIdsFirstPart); - Assertions.assertThrows(RedisException.class, () -> s.release(permitsIdsThirdPart)); + List permitsIdsSecondPart = permitsIds.subList(2, 4); + permitsIdsSecondPart.addAll(permitsIdsFirstPart); + Assertions.assertThrows(RedisException.class, () -> s.release(permitsIdsSecondPart)); assertThat(s.availablePermits()).isEqualTo(4); - - } @Test From 8e06ee17fdd7d296feef759e716803e46ee71330 Mon Sep 17 00:00:00 2001 From: seakider Date: Wed, 18 Dec 2024 22:13:28 +0800 Subject: [PATCH 3/4] Fixed - RPermitExpirableSemaphore.release(java.util.List) shouldn't release permits if one of them doesn't exist #6343 Signed-off-by: xuxiaolei --- .../redisson/RedissonPermitExpirableSemaphoreTest.java | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/redisson/src/test/java/org/redisson/RedissonPermitExpirableSemaphoreTest.java b/redisson/src/test/java/org/redisson/RedissonPermitExpirableSemaphoreTest.java index 18ec8e6de..600f71c9d 100644 --- a/redisson/src/test/java/org/redisson/RedissonPermitExpirableSemaphoreTest.java +++ b/redisson/src/test/java/org/redisson/RedissonPermitExpirableSemaphoreTest.java @@ -1,5 +1,6 @@ package org.redisson; +import net.bytebuddy.utility.RandomString; import org.awaitility.Awaitility; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; @@ -577,6 +578,14 @@ public class RedissonPermitExpirableSemaphoreTest extends BaseConcurrentTest { permitsIdsSecondPart.addAll(permitsIdsFirstPart); Assertions.assertThrows(RedisException.class, () -> s.release(permitsIdsSecondPart)); assertThat(s.availablePermits()).isEqualTo(4); + + Assertions.assertThrows(RedisException.class, () -> s.release(List.of("1234"))); + assertThat(s.availablePermits()).isEqualTo(4); + + List permitsIdsThirdPart = permitsIds.subList(4, 6); + permitsIdsThirdPart.add("4567"); + Assertions.assertThrows(RedisException.class, () -> s.release(permitsIdsThirdPart)); + assertThat(s.availablePermits()).isEqualTo(4); } @Test From 7451a972d0f118d269b4a6f7128848bf473d0e85 Mon Sep 17 00:00:00 2001 From: seakider Date: Wed, 18 Dec 2024 22:15:23 +0800 Subject: [PATCH 4/4] Fixed - RPermitExpirableSemaphore.release(java.util.List) shouldn't release permits if one of them doesn't exist #6343 Signed-off-by: xuxiaolei --- .../java/org/redisson/RedissonPermitExpirableSemaphoreTest.java | 1 - 1 file changed, 1 deletion(-) diff --git a/redisson/src/test/java/org/redisson/RedissonPermitExpirableSemaphoreTest.java b/redisson/src/test/java/org/redisson/RedissonPermitExpirableSemaphoreTest.java index 600f71c9d..89d3b05ba 100644 --- a/redisson/src/test/java/org/redisson/RedissonPermitExpirableSemaphoreTest.java +++ b/redisson/src/test/java/org/redisson/RedissonPermitExpirableSemaphoreTest.java @@ -1,6 +1,5 @@ package org.redisson; -import net.bytebuddy.utility.RandomString; import org.awaitility.Awaitility; import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test;