From 15fc23c2fa4565dce9d02a09d32cba10df5f22ff Mon Sep 17 00:00:00 2001 From: Artem Bilan Date: Tue, 17 Apr 2018 17:05:09 -0400 Subject: [PATCH] INT-4447: LockRegLeaderInit: Catch unlock errors JIRA: https://jira.spring.io/browse/INT-4447 When we get an exception during `this.lock.unlock()`, we don't revoke leadership. In case of external resource (e.g. JDBC) this may cause a race condition when the second candidate is selected as leader when connection comes back * Catch `this.lock.unlock()` exceptions and log them under DEBUG. This way we proceed to the `handleRevoked()` logic **Cherry-pick to 5.0.x and 4.3.x** --- .../leader/LockRegistryLeaderInitiator.java | 18 ++++++++++-- .../JdbcLockRegistryLeaderInitiatorTests.java | 29 +++++++++++++++++++ 2 files changed, 44 insertions(+), 3 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 12b1b98f90..9e4a169209 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 @@ -381,8 +381,15 @@ public class LockRegistryLeaderInitiator implements SmartLifecycle, DisposableBe } catch (Exception e) { if (this.locked) { - this.lock.unlock(); this.locked = false; + try { + this.lock.unlock(); + } + catch (Exception e1) { + logger.debug("Could not unlock - treat as broken. " + + "Revoking " + (isRunning() ? " and retrying..." : "..."), e); + + } // The lock was broken and we are no longer leader handleRevoked(); if (isRunning()) { @@ -408,11 +415,16 @@ public class LockRegistryLeaderInitiator implements SmartLifecycle, DisposableBe } finally { if (this.locked) { - this.lock.unlock(); + this.locked = false; + try { + this.lock.unlock(); + } + catch (Exception e) { + logger.debug("Could not unlock during stop - treat as broken. Revoking...", e); + } // We are stopping, therefore not leading any more handleRevoked(); } - this.locked = false; } return null; } diff --git a/spring-integration-jdbc/src/test/java/org/springframework/integration/jdbc/leader/JdbcLockRegistryLeaderInitiatorTests.java b/spring-integration-jdbc/src/test/java/org/springframework/integration/jdbc/leader/JdbcLockRegistryLeaderInitiatorTests.java index 7592427854..21bdccecfa 100644 --- a/spring-integration-jdbc/src/test/java/org/springframework/integration/jdbc/leader/JdbcLockRegistryLeaderInitiatorTests.java +++ b/spring-integration-jdbc/src/test/java/org/springframework/integration/jdbc/leader/JdbcLockRegistryLeaderInitiatorTests.java @@ -167,6 +167,35 @@ public class JdbcLockRegistryLeaderInitiatorTests { initiator1.stop(); } + @Test + public void testLostConnection() throws InterruptedException { + CountDownLatch granted = new CountDownLatch(1); + CountingPublisher countingPublisher = new CountingPublisher(granted); + + DefaultLockRepository lockRepository = new DefaultLockRepository(dataSource); + lockRepository.afterPropertiesSet(); + LockRegistryLeaderInitiator initiator = new LockRegistryLeaderInitiator(new JdbcLockRegistry(lockRepository)); + initiator.setLeaderEventPublisher(countingPublisher); + + initiator.start(); + + assertThat(granted.await(10, TimeUnit.SECONDS), is(true)); + + destroy(); + + assertThat(countingPublisher.revoked.await(10, TimeUnit.SECONDS), is(true)); + + granted = new CountDownLatch(1); + countingPublisher = new CountingPublisher(granted); + initiator.setLeaderEventPublisher(countingPublisher); + + init(); + + assertThat(granted.await(10, TimeUnit.SECONDS), is(true)); + + initiator.stop(); + } + private static class CountingPublisher implements LeaderEventPublisher { private final CountDownLatch granted;