Skip to content

Commit 9d73762

Browse files
artembilangaryrussell
authored andcommitted
GH-3888: Fix NPE in the RedisLockRegistry.destroy() (#3889)
* GH-3888: Fix NPE in the `RedisLockRegistry.destroy()` Fixed #3888 When `RedisLockType.SPIN_LOCK` (default), the `RedisLockRegistry.destroy()` causes an NPE on the `redisMessageListenerContainer` since pub-sub is not used in a busy-spin mode * Check for `redisMessageListenerContainer` before calling its `destroy()` **Cherry-pick to 5.5.x** * * Reset properties in the `RedisLockRegistry` after `destroy()`
1 parent 63109f7 commit 9d73762

File tree

2 files changed

+37
-5
lines changed

2 files changed

+37
-5
lines changed

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

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -245,11 +245,15 @@ public void destroy() {
245245
if (!this.executorExplicitlySet) {
246246
((ExecutorService) this.executor).shutdown();
247247
}
248-
try {
249-
this.redisMessageListenerContainer.destroy();
250-
}
251-
catch (Exception ex) {
252-
throw new IllegalStateException(ex);
248+
if (this.redisMessageListenerContainer != null) {
249+
try {
250+
this.redisMessageListenerContainer.destroy();
251+
this.redisMessageListenerContainer = null;
252+
this.isRunningRedisMessageListenerContainer = false;
253+
}
254+
catch (Exception ex) {
255+
throw new IllegalStateException(ex);
256+
}
253257
}
254258
}
255259

spring-integration-redis/src/test/java/org/springframework/integration/redis/util/RedisLockRegistryTests.java

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,7 @@ public void testLock() {
120120
}
121121
registry.expireUnusedOlderThan(-1000);
122122
assertThat(TestUtils.getPropertyValue(registry, "locks", Map.class).size()).isEqualTo(0);
123+
registry.destroy();
123124
}
124125

125126
@Test
@@ -139,6 +140,7 @@ public void testLockInterruptibly() throws Exception {
139140
}
140141
registry.expireUnusedOlderThan(-1000);
141142
assertThat(TestUtils.getPropertyValue(registry, "locks", Map.class).size()).isEqualTo(0);
143+
registry.destroy();
142144
}
143145

144146
@Test
@@ -166,6 +168,7 @@ public void testReentrantLock() {
166168
}
167169
registry.expireUnusedOlderThan(-1000);
168170
assertThat(TestUtils.getPropertyValue(registry, "locks", Map.class).size()).isEqualTo(0);
171+
registry.destroy();
169172
}
170173

171174
@Test
@@ -193,6 +196,7 @@ public void testReentrantLockInterruptibly() throws Exception {
193196
}
194197
registry.expireUnusedOlderThan(-1000);
195198
assertThat(TestUtils.getPropertyValue(registry, "locks", Map.class).size()).isEqualTo(0);
199+
registry.destroy();
196200
}
197201

198202
@Test
@@ -220,6 +224,7 @@ public void testTwoLocks() throws Exception {
220224
}
221225
registry.expireUnusedOlderThan(-1000);
222226
assertThat(TestUtils.getPropertyValue(registry, "locks", Map.class).size()).isEqualTo(0);
227+
registry.destroy();
223228
}
224229

225230
@Test
@@ -251,6 +256,7 @@ public void testTwoThreadsSecondFailsToGetLock() throws Exception {
251256
assertThat(((Exception) ise).getMessage()).contains("You do not own lock at");
252257
registry.expireUnusedOlderThan(-1000);
253258
assertThat(TestUtils.getPropertyValue(registry, "locks", Map.class).size()).isEqualTo(0);
259+
registry.destroy();
254260
}
255261

256262
@Test
@@ -290,6 +296,7 @@ public void testTwoThreads() throws Exception {
290296
assertThat(locked.get()).isTrue();
291297
registry.expireUnusedOlderThan(-1000);
292298
assertThat(TestUtils.getPropertyValue(registry, "locks", Map.class).size()).isEqualTo(0);
299+
registry.destroy();
293300
}
294301

295302
@Test
@@ -339,6 +346,8 @@ public void testTwoThreadsDifferentRegistries() throws Exception {
339346
registry2.expireUnusedOlderThan(-1000);
340347
assertThat(TestUtils.getPropertyValue(registry1, "locks", Map.class).size()).isEqualTo(0);
341348
assertThat(TestUtils.getPropertyValue(registry2, "locks", Map.class).size()).isEqualTo(0);
349+
registry1.destroy();
350+
registry2.destroy();
342351
}
343352

344353
@Test
@@ -368,6 +377,7 @@ public void testTwoThreadsWrongOneUnlocks() throws Exception {
368377
assertThat(((Exception) ise).getMessage()).contains("You do not own lock at");
369378
registry.expireUnusedOlderThan(-1000);
370379
assertThat(TestUtils.getPropertyValue(registry, "locks", Map.class).size()).isEqualTo(0);
380+
registry.destroy();
371381
}
372382

373383
@Test
@@ -384,6 +394,8 @@ public void testExpireTwoRegistries() throws Exception {
384394
waitForExpire("foo");
385395
assertThat(lock2.tryLock()).isTrue();
386396
assertThat(lock1.tryLock()).isFalse();
397+
registry1.destroy();
398+
registry2.destroy();
387399
}
388400

389401
@Test
@@ -397,6 +409,7 @@ public void testExceptionOnExpire() throws Exception {
397409
assertThatIllegalStateException()
398410
.isThrownBy(lock1::unlock)
399411
.withMessageContaining("Lock was released in the store due to expiration.");
412+
registry.destroy();
400413
}
401414

402415

@@ -435,6 +448,9 @@ public void testEquals() {
435448
lock2.lock();
436449
lock1.unlock();
437450
lock2.unlock();
451+
registry1.destroy();
452+
registry2.destroy();
453+
registry3.destroy();
438454
}
439455

440456
@Test
@@ -459,6 +475,7 @@ public void testThreadLocalListLeaks() {
459475
lock.unlock();
460476
}
461477
assertThat(TestUtils.getPropertyValue(registry, "locks", Map.class).size()).isEqualTo(10);
478+
registry.destroy();
462479
}
463480

464481
@Test
@@ -481,6 +498,7 @@ public void testExpireNotChanged() throws Exception {
481498
result.get();
482499
assertThat(getExpire(registry, "foo")).isEqualTo(expire);
483500
lock.unlock();
501+
registry.destroy();
484502
}
485503

486504
@Test
@@ -523,6 +541,7 @@ public void concurrentObtainCapacityTest() throws InterruptedException {
523541

524542
registry.expireUnusedOlderThan(-1000);
525543
assertThat(TestUtils.getPropertyValue(registry, "locks", Map.class).size()).isEqualTo(0);
544+
registry.destroy();
526545
}
527546

528547
@Test
@@ -572,6 +591,7 @@ public void concurrentObtainRemoveOrderTest() throws InterruptedException {
572591

573592
assertThat(getRedisLockRegistryLocks(registry)).containsKeys(
574593
remainLockCheckQueue.toArray(new String[remainLockCheckQueue.size()]));
594+
registry.destroy();
575595
}
576596

577597
@Test
@@ -627,6 +647,7 @@ public void concurrentObtainAccessRemoveOrderTest() throws InterruptedException
627647

628648
assertThat(getRedisLockRegistryLocks(registry)).containsKeys(
629649
remainLockCheckQueue.toArray(new String[remainLockCheckQueue.size()]));
650+
registry.destroy();
630651
}
631652

632653
@Test
@@ -655,6 +676,7 @@ public void setCapacityTest() {
655676
registry.obtain("foo:5");
656677
assertThat(TestUtils.getPropertyValue(registry, "locks", Map.class).size()).isEqualTo(4);
657678
assertThat(getRedisLockRegistryLocks(registry)).containsKeys("foo:3", "foo:4", "foo:5");
679+
registry.destroy();
658680
}
659681

660682
@RedisAvailable
@@ -701,6 +723,8 @@ public void twoRedisLockRegistryTest() throws InterruptedException {
701723
});
702724

703725
endDownLatch.await();
726+
registry1.destroy();
727+
registry2.destroy();
704728
}
705729

706730
@RedisAvailable
@@ -795,6 +819,9 @@ public void earlyWakeUpTest() throws InterruptedException {
795819
assertThat(awaitTimeout.await(1, TimeUnit.SECONDS)).isFalse();
796820
assertThat(expectOne.get()).isEqualTo(1);
797821
executorService.shutdown();
822+
registry1.destroy();
823+
registry2.destroy();
824+
registry3.destroy();
798825
}
799826

800827

@@ -812,6 +839,7 @@ public void testUlink() {
812839
registry = new RedisLockRegistry(mock(RedisConnectionFactory.class), "foo");
813840
registry.setRedisLockType(testRedisLockType);
814841
assertThat(TestUtils.getPropertyValue(registry, "ulinkAvailable", Boolean.class)).isTrue();
842+
registry.destroy();
815843
}
816844

817845
private Long getExpire(RedisLockRegistry registry, String lockKey) {

0 commit comments

Comments
 (0)