Skip to content

Commit c37a247

Browse files
Fix borrowObject timeout wrapping (#3666)
* Fix borrowObject timeout wrapping #3623 Override borrowObject(long borrowMaxWaitMillis) in GenericObjectPool to apply connection wrapping when borrowing with timeout. This ensures connections borrowed with timeout are properly returned to the pool when closed via try-with-resources. Without this fix, borrowObject(timeout) returns unwrapped connections that don't auto-return to pool, causing connection leaks. * Add borrowObject(Duration) override and test #3623 Override borrowObject(Duration) in GenericObjectPool to apply connection wrapping, ensuring connections borrowed with Duration timeout are properly returned to pool when closed via try-with-resources. Add integration test following the same pattern as borrowObject(long) test. * Rename test methods for clarity Rename borrowObject test methods to be more explicit: - genericPoolShouldWorkWithBorrowObjectTimeout -> TimeoutMillis - genericPoolShouldWorkWithBorrowObjectDuration -> TimeoutDuration This makes it clearer which parameter type each test covers. * Fix tests * Override only the Duration borrow method --------- Co-authored-by: aleksandar.todorov <a_t_todorov@yahoo.com>
1 parent ec2f2b9 commit c37a247

2 files changed

Lines changed: 39 additions & 3 deletions

File tree

src/main/java/io/lettuce/core/support/ConnectionPoolSupport.java

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
import static io.lettuce.core.support.ConnectionWrapping.HasTargetConnection;
44

5+
import java.time.Duration;
56
import java.util.concurrent.CompletableFuture;
67
import java.util.concurrent.atomic.AtomicReference;
78
import java.util.concurrent.locks.Lock;
@@ -62,6 +63,7 @@
6263
*
6364
* @author Mark Paluch
6465
* @author dae won
66+
* @author JiHongKim98
6567
* @since 4.3
6668
*/
6769
public abstract class ConnectionPoolSupport {
@@ -144,9 +146,10 @@ private ConnectionPoolSupport() {
144146
new RedisPooledObjectFactory<>(connectionSupplier, validationPredicate), config) {
145147

146148
@Override
147-
public T borrowObject() throws Exception {
148-
return wrapConnections ? ConnectionWrapping.wrapConnection(super.borrowObject(), poolRef.get())
149-
: super.borrowObject();
149+
public T borrowObject(Duration borrowMaxWaitDuration) throws Exception {
150+
return wrapConnections
151+
? ConnectionWrapping.wrapConnection(super.borrowObject(borrowMaxWaitDuration), poolRef.get())
152+
: super.borrowObject(borrowMaxWaitDuration);
150153
}
151154

152155
@Override

src/test/java/io/lettuce/core/support/ConnectionPoolSupportIntegrationTests.java

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
import static org.assertj.core.api.Assertions.fail;
66

77
import java.lang.reflect.Proxy;
8+
import java.time.Duration;
89
import java.util.Set;
910

1011
import org.apache.commons.pool2.ObjectPool;
@@ -406,6 +407,38 @@ void tryWithResourcesReturnsSoftRefConnectionToPool() throws Exception {
406407
pool.close();
407408
}
408409

410+
@Test
411+
void genericPoolShouldWorkWithBorrowObjectTimeoutMillis() throws Exception {
412+
413+
GenericObjectPool<StatefulRedisConnection<String, String>> pool = ConnectionPoolSupport
414+
.createGenericObjectPool(() -> client.connect(), new GenericObjectPoolConfig<>());
415+
416+
try (StatefulRedisConnection<String, String> connection = pool.borrowObject(10_000)) {
417+
RedisCommands<String, String> sync = connection.sync();
418+
sync.ping();
419+
}
420+
421+
assertThat(pool.getNumActive()).isEqualTo(0);
422+
423+
pool.close();
424+
}
425+
426+
@Test
427+
void genericPoolShouldWorkWithBorrowObjectTimeoutDuration() throws Exception {
428+
429+
GenericObjectPool<StatefulRedisConnection<String, String>> pool = ConnectionPoolSupport
430+
.createGenericObjectPool(() -> client.connect(), new GenericObjectPoolConfig<>());
431+
432+
try (StatefulRedisConnection<String, String> connection = pool.borrowObject(Duration.ofSeconds(10))) {
433+
RedisCommands<String, String> sync = connection.sync();
434+
sync.ping();
435+
}
436+
437+
assertThat(pool.getNumActive()).isEqualTo(0);
438+
439+
pool.close();
440+
}
441+
409442
private void borrowAndReturn(ObjectPool<StatefulRedisConnection<String, String>> pool) throws Exception {
410443

411444
for (int i = 0; i < 10; i++) {

0 commit comments

Comments
 (0)