From c9faf3c8ce3daedab56c46835dd48430c39b3b2e Mon Sep 17 00:00:00 2001 From: Artem Bilan Date: Wed, 30 May 2018 15:21:51 -0400 Subject: [PATCH] Fix LockRegLeaderIn for interrupted Thread.sleep (#2455) * Fix LockRegLeaderIn for interrupted Thread.sleep https://build.spring.io/browse/INT-FATS5IC-517 If current thread is interrupted, the `Thread.sleep()` interrupts immediately. In the catch block of the main loop in the `LockRegistryLeaderInitiator` we have such a dangerous `sleep()` and don't restart election in this candidate any more * Move `Thread.sleep()` to else after checking the current thread for interrupted state * Remove `LongRunningIntegrationTest` rule from the `RedisLockRegistryLeaderInitiatorTests` since it now works much faster after proper `busy-wait` handling **Cherry-pick to master** * Ignore interruption on the sleep i catch and move on with loop --- .../leader/LockRegistryLeaderInitiator.java | 22 +++++++++++++------ ...RedisLockRegistryLeaderInitiatorTests.java | 4 ---- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/spring-integration-core/src/main/java/org/springframework/integration/support/leader/LockRegistryLeaderInitiator.java b/spring-integration-core/src/main/java/org/springframework/integration/support/leader/LockRegistryLeaderInitiator.java index 4e2a6cc27b..0acfa4dda0 100644 --- a/spring-integration-core/src/main/java/org/springframework/integration/support/leader/LockRegistryLeaderInitiator.java +++ b/spring-integration-core/src/main/java/org/springframework/integration/support/leader/LockRegistryLeaderInitiator.java @@ -397,10 +397,6 @@ public class LockRegistryLeaderInitiator implements SmartLifecycle, DisposableBe } // The lock was broken and we are no longer leader handleRevoked(); - if (isRunning()) { - // Give it a chance to elect some other leader. - Thread.sleep(LockRegistryLeaderInitiator.this.busyWaitMillis); - } } if (e instanceof InterruptedException || Thread.currentThread().isInterrupted()) { @@ -417,9 +413,21 @@ public class LockRegistryLeaderInitiator implements SmartLifecycle, DisposableBe } return null; } - else if (logger.isDebugEnabled()) { - logger.debug("Error acquiring the lock for " + this.context + - ". " + (isRunning() ? "Retrying..." : ""), e); + else { + if (isRunning()) { + // Give it a chance to elect some other leader. + try { + Thread.sleep(LockRegistryLeaderInitiator.this.busyWaitMillis); + } + catch (InterruptedException e1) { + // Ignore interruption and let it to be caught on the next cycle. + Thread.currentThread().interrupt(); + } + } + if (logger.isDebugEnabled()) { + logger.debug("Error acquiring the lock for " + this.context + + ". " + (isRunning() ? "Retrying..." : ""), e); + } } } } diff --git a/spring-integration-redis/src/test/java/org/springframework/integration/redis/leader/RedisLockRegistryLeaderInitiatorTests.java b/spring-integration-redis/src/test/java/org/springframework/integration/redis/leader/RedisLockRegistryLeaderInitiatorTests.java index 86a8d2640b..316dfc5b38 100644 --- a/spring-integration-redis/src/test/java/org/springframework/integration/redis/leader/RedisLockRegistryLeaderInitiatorTests.java +++ b/spring-integration-redis/src/test/java/org/springframework/integration/redis/leader/RedisLockRegistryLeaderInitiatorTests.java @@ -37,7 +37,6 @@ import org.springframework.integration.redis.rules.RedisAvailableTests; import org.springframework.integration.redis.util.RedisLockRegistry; import org.springframework.integration.support.leader.LockRegistryLeaderInitiator; import org.springframework.integration.test.rule.Log4j2LevelAdjuster; -import org.springframework.integration.test.support.LongRunningIntegrationTest; import org.springframework.scheduling.concurrent.CustomizableThreadFactory; /** @@ -49,9 +48,6 @@ import org.springframework.scheduling.concurrent.CustomizableThreadFactory; */ public class RedisLockRegistryLeaderInitiatorTests extends RedisAvailableTests { - @Rule - public LongRunningIntegrationTest longTests = new LongRunningIntegrationTest(); - @Rule public Log4j2LevelAdjuster adjuster = Log4j2LevelAdjuster.trace()