GH-11041: Revise cache in the JdbcLockRegistry

Fixes: #11041

* Fix `JdbcLockRegistry.renewLock()` to rely on the `obtain()`

The held lock might be evicted from the cache,
so the null from the `this.locks.get(path)` does not reflect reality

* Explain more about `cacheCapacity` in the docs

# Conflicts:
#	spring-integration-jdbc/src/main/java/org/springframework/integration/jdbc/lock/JdbcLockRegistry.java
#	spring-integration-jdbc/src/test/java/org/springframework/integration/jdbc/lock/JdbcLockRegistryTests.java
#	src/reference/antora/modules/ROOT/pages/jdbc/lock-registry.adoc

Author: artembilan

spring-integration-jdbc/src/main/java/org/springframework/integration/jdbc/lock/JdbcLockRegistry.java

@@ -29,6 +29,7 @@
 import org.springframework.dao.CannotAcquireLockException;
 import org.springframework.dao.DataAccessResourceFailureException;
 import org.springframework.dao.TransientDataAccessException;
+import org.springframework.integration.support.locks.DefaultLockRegistry;
 import org.springframework.integration.support.locks.ExpirableLockRegistry;
 import org.springframework.integration.support.locks.RenewableLockRegistry;
 import org.springframework.integration.util.UUIDConverter;
@@ -65,7 +66,7 @@ public class JdbcLockRegistry implements ExpirableLockRegistry, RenewableLockReg
 
 	private static final int DEFAULT_IDLE = 100;
 
-	private static final int DEFAULT_CAPACITY = 100_000;
+	private static final int DEFAULT_CAPACITY = 256;
 
 	private final Lock lock = new ReentrantLock();
 
@@ -85,6 +86,8 @@ protected boolean removeEldestEntry(Entry<String, JdbcLock> eldest) {
 
 	private int cacheCapacity = DEFAULT_CAPACITY;
 
+	private DefaultLockRegistry defaultLockRegistry = new DefaultLockRegistry();
+
 	/**
 	 * Construct an instance based on the provided {@link LockRepository}.
 	 * @param client the {@link LockRepository} to rely on.
@@ -106,11 +109,15 @@ public void setIdleBetweenTries(Duration idleBetweenTries) {
 
 	/**
 	 * Set the capacity of cached locks.
-	 * @param cacheCapacity The capacity of cached lock, (default 100_000).
+	 * @param cacheCapacity The capacity of cached lock, (default 256 locks).
 	 * @since 5.5.6
+	 * @see DefaultLockRegistry
 	 */
 	public void setCacheCapacity(int cacheCapacity) {
 		this.cacheCapacity = cacheCapacity;
+		// Find the highest power of 2 for (n + 1), then subtract 1
+		int mask = Integer.highestOneBit(cacheCapacity + 1) - 1;
+		this.defaultLockRegistry = new DefaultLockRegistry(mask);
 	}
 
 	@Override
@@ -148,26 +155,13 @@ public void expireUnusedOlderThan(long age) {
 
 	@Override
 	public void renewLock(Object lockKey) {
-		Assert.isInstanceOf(String.class, lockKey);
-		String path = pathFor((String) lockKey);
-		JdbcLock jdbcLock;
-		this.lock.lock();
-		try {
-			jdbcLock = this.locks.get(path);
-		}
-		finally {
-			this.lock.unlock();
-		}
-
-		if (jdbcLock == null) {
-			throw new IllegalStateException("Could not found mutex at " + path);
-		}
+		JdbcLock jdbcLock = (JdbcLock) obtain(lockKey);
 		if (!jdbcLock.renew()) {
-			throw new IllegalStateException("Could not renew mutex at " + path);
+			throw new IllegalStateException("Could not renew lock " + lockKey);
 		}
 	}
 
-	private static final class JdbcLock implements Lock {
+	private final class JdbcLock implements Lock {
 
 		private final LockRepository mutex;
 
@@ -177,12 +171,13 @@ private static final class JdbcLock implements Lock {
 
 		private volatile long lastUsed = System.currentTimeMillis();
 
-		private final ReentrantLock delegate = new ReentrantLock();
+		private final ReentrantLock delegate;
 
 		JdbcLock(LockRepository client, Duration idleBetweenTries, String path) {
 			this.mutex = client;
 			this.idleBetweenTries = idleBetweenTries;
 			this.path = path;
+			this.delegate = (ReentrantLock) JdbcLockRegistry.this.defaultLockRegistry.obtain(this.path);
 		}
 
 		public long getLastUsed() {

spring-integration-jdbc/src/test/java/org/springframework/integration/jdbc/lock/JdbcLockRegistryTests.java

@@ -47,6 +47,7 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
+import static org.assertj.core.api.Assertions.assertThatNoException;
 
 /**
  * @author Dave Syer
@@ -553,6 +554,53 @@ void testUnlockAfterLockStatusHasBeenExpiredAndDeleted() throws Exception {
 				.isThrownBy(lock::unlock);
 	}
 
+	@Test
+	void noSecondLockOnEviction() throws InterruptedException {
+		DefaultLockRepository client = new DefaultLockRepository(dataSource);
+		client.setApplicationContext(this.context);
+		client.afterPropertiesSet();
+		client.afterSingletonsInstantiated();
+		JdbcLockRegistry registry = new JdbcLockRegistry(client);
+		registry.setCacheCapacity(2);
+
+		CountDownLatch lock1Latch = new CountDownLatch(1);
+		CountDownLatch furtherLocksLatch = new CountDownLatch(1);
+
+		Executors.newSingleThreadExecutor()
+				.execute(() -> {
+					Lock lock1 = registry.obtain("lock1");
+					lock1.lock();
+					try {
+						furtherLocksLatch.countDown();
+						lock1Latch.await(10, TimeUnit.SECONDS);
+					}
+					catch (InterruptedException e) {
+						Thread.currentThread().interrupt();
+					}
+					finally {
+						lock1.unlock();
+					}
+				});
+
+		assertThat(furtherLocksLatch.await(10, TimeUnit.SECONDS)).isTrue();
+		// Two new locks to trigger cache eviction for the 'lock1'
+		registry.obtain("lock2");
+		registry.obtain("lock3");
+
+		// Request 'lock1' again: will trigger new JdbcLock instance
+		Lock lock1 = registry.obtain("lock1");
+
+		// Cannot lock because 'lock1' is still locked by another thread
+		assertThat(lock1.tryLock(1, TimeUnit.SECONDS)).isFalse();
+
+		lock1Latch.countDown();
+
+		assertThatNoException().isThrownBy(() -> {
+			lock1.lock();
+			lock1.unlock();
+		});
+	}
+
 	@SuppressWarnings("unchecked")
 	private static Map<String, Lock> getRegistryLocks(JdbcLockRegistry registry) {
 		return TestUtils.getPropertyValue(registry, "locks", Map.class);

src/reference/antora/modules/ROOT/pages/jdbc/lock-registry.adoc

@@ -43,8 +43,11 @@ So the time to live can be highly reduce and deployments can retake a lost lock
 
 NOTE: The lock renewal can be done only if the lock is held by the current thread.
 
-Starting with version 5.5.6, the `JdbcLockRegistry` is support automatically clean up cache for JdbcLock in `JdbcLockRegistry.locks` via `JdbcLockRegistry.setCacheCapacity()`.
-See its JavaDocs for more information.
+Starting with version 5.5.6, the `JdbcLockRegistry` is support automatically clean-up cache for JdbcLock in `JdbcLockRegistry.locks` via `JdbcLockRegistry.setCacheCapacity()`.
+The default value is `256`.
+See its Javadocs for more information.
+This capacity number is also propagated into the `DefaultLockRegistry` used internally in the `JdbcLockRegistry` for a pool of shared `ReentrantLock` instances.
+So, if higher concurrent access (the number of unique lock keys used in parallel) is expected (or allowed) for the application, the `cacheCapacity` should be increased respectively, with the closest power of 2 in mind (essentially `Integer.highestOneBit(cacheCapacity + 1)`).
 
 Starting with version 6.0, the `DefaultLockRepository` can be supplied with a `PlatformTransactionManager` instead of relying on the primary bean from the application context.
 
@@ -58,4 +61,4 @@ lockRepository.setInsertQuery(lockRepository.getInsertQuery() + " ON CONFLICT DO
 ----
 
 Starting with version 6.4, the `LockRepository.delete()` method return the result of removing ownership of a distributed lock.
-And the `JdbcLockRegistry.JdbcLock.unlock()` method throws `ConcurrentModificationException` if the ownership of the lock is expired.
+And the `JdbcLockRegistry.JdbcLock.unlock()` method throws `ConcurrentModificationException` if the ownership of the lock is expired.