Fix connection leak in scanIteration with JedisSentineled #4323 (#4328)

* docs: document required optional dependency `resilience4j-all` when using failover client

* Fix connection leak in scanIteration with JedisSentineled

Override getConnectionMap() in SentineledConnectionProvider to return
the pool instead of a raw connection. This prevents connection leaks
when using scanIteration() with JedisSentineled.

Fixes #4323

Author: ggivo

src/main/java/redis/clients/jedis/providers/SentineledConnectionProvider.java

@@ -2,7 +2,9 @@
 
 import java.util.ArrayList;
 import java.util.Collection;
+import java.util.Collections;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.locks.Lock;
@@ -24,6 +26,7 @@
 import redis.clients.jedis.exceptions.JedisConnectionException;
 import redis.clients.jedis.exceptions.JedisException;
 import redis.clients.jedis.util.IOUtils;
+import redis.clients.jedis.util.Pool;
 
 public class SentineledConnectionProvider implements ConnectionProvider {
 
@@ -112,6 +115,16 @@ public Connection getConnection(CommandArguments args) {
     return pool.getResource();
   }
 
+  @Override
+  public Map<?, Pool<Connection>> getConnectionMap() {
+    return Collections.singletonMap(currentMaster, pool);
+  }
+
+  @Override
+  public Map<?, Pool<Connection>> getPrimaryNodesConnectionMap() {
+    return Collections.singletonMap(currentMaster, pool);
+  }
+
   @Override
   public void close() {
     sentinelListeners.forEach(SentinelListener::shutdown);

src/test/java/redis/clients/jedis/SentineledConnectionProviderTest.java

@@ -1,17 +1,24 @@
 package redis.clients.jedis;
 
 import java.util.HashSet;
+import java.util.Map;
 import java.util.Set;
 
 import org.apache.commons.pool2.impl.GenericObjectPoolConfig;
 
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Timeout;
 import redis.clients.jedis.exceptions.JedisConnectionException;
 import redis.clients.jedis.exceptions.JedisException;
 import redis.clients.jedis.providers.SentineledConnectionProvider;
+import redis.clients.jedis.util.ReflectionTestUtil;
 
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.equalTo;
+import static org.hamcrest.Matchers.hasKey;
+import static org.hamcrest.Matchers.sameInstance;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertSame;
 import static org.junit.jupiter.api.Assertions.assertThrows;
@@ -28,6 +35,8 @@ public class SentineledConnectionProviderTest {
   protected static final HostAndPort sentinel1 = HostAndPorts.getSentinelServers().get(1);
   protected static final HostAndPort sentinel2 = HostAndPorts.getSentinelServers().get(3);
 
+  private static final EndpointConfig primary = HostAndPorts.getRedisEndpoint("standalone2-primary");
+
   protected Set<HostAndPort> sentinels = new HashSet<>();
 
   @BeforeEach
@@ -51,6 +60,68 @@ public void repeatedSentinelPoolInitialization() {
     }
   }
 
+  /**
+   * Ensure that getConnectionMap() does not cause connection leak. (#4323)
+   */
+  @Test
+  @Timeout( value = 1)
+  public void getConnectionMapDoesNotCauseConnectionLeak() {
+
+    ConnectionPoolConfig config = new ConnectionPoolConfig();
+    config.setMaxTotal(1);
+
+    try (SentineledConnectionProvider sut = new SentineledConnectionProvider(MASTER_NAME,
+            primary.getClientConfigBuilder().build(), config, sentinels,
+            DefaultJedisClientConfig.builder().build())) {
+
+      HostAndPort resolvedPrimary = sut.getCurrentMaster();
+      ConnectionPool pool = ReflectionTestUtil.getField(sut,"pool");
+      assertThat(pool.getNumActive(), equalTo(0));
+
+      Map<?, ?> cm = sut.getConnectionMap();
+
+      // exactly one entry for current primary
+      // and no active connections
+      assertThat(cm.size(), equalTo(1));
+      assertThat(cm, hasKey(resolvedPrimary));
+      assertThat(pool.getNumActive(), equalTo(0));
+      // primary did not change
+      assertThat(ReflectionTestUtil.getField(sut,"pool"), sameInstance(pool));
+    }
+  }
+
+  /**
+   * Ensure that getPrimaryNodesConnectionMap() does not cause connection leak. (#4323)
+   */
+  @Test
+  @Timeout( value = 1)
+  public void getPrimaryNodesConnectionMapDoesNotCauseConnectionLeak() {
+
+    ConnectionPoolConfig config = new ConnectionPoolConfig();
+    config.setMaxTotal(1);
+
+    try (SentineledConnectionProvider sut = new SentineledConnectionProvider(MASTER_NAME,
+            primary.getClientConfigBuilder().build(), config, sentinels,
+            DefaultJedisClientConfig.builder().build())) {
+
+      HostAndPort resolvedPrimary = sut.getCurrentMaster();
+      ConnectionPool pool = ReflectionTestUtil.getField(sut,"pool");
+      assertThat(pool.getNumActive(), equalTo(0));
+
+
+      Map<?, ?> cm = sut.getPrimaryNodesConnectionMap();
+
+      // exactly one entry for current primary
+      // and no active connections
+      assertThat(cm.size(), equalTo(1));
+      assertThat(cm, hasKey(resolvedPrimary));
+      assertThat(pool.getNumActive(), equalTo(0));
+      // primary did not change
+      assertThat(ReflectionTestUtil.getField(sut,"pool"), sameInstance(pool));
+    }
+
+  }
+
   @Test
   public void initializeWithNotAvailableSentinelsShouldThrowException() {
     Set<HostAndPort> wrongSentinels = new HashSet<>();

src/test/java/redis/clients/jedis/commands/unified/sentinel/SentinelAllKindOfValuesCommandsIT.java

@@ -0,0 +1,57 @@
+package redis.clients.jedis.commands.unified.sentinel;
+
+import org.junit.jupiter.api.extension.RegisterExtension;
+import org.junit.jupiter.params.ParameterizedClass;
+import org.junit.jupiter.params.provider.MethodSource;
+import redis.clients.jedis.DefaultJedisClientConfig;
+import redis.clients.jedis.EndpointConfig;
+import redis.clients.jedis.HostAndPort;
+import redis.clients.jedis.HostAndPorts;
+import redis.clients.jedis.JedisClientConfig;
+import redis.clients.jedis.JedisSentineled;
+import redis.clients.jedis.RedisProtocol;
+import redis.clients.jedis.UnifiedJedis;
+import redis.clients.jedis.commands.unified.AllKindOfValuesCommandsTestBase;
+import redis.clients.jedis.util.EnabledOnCommandCondition;
+import redis.clients.jedis.util.RedisVersionCondition;
+
+import java.util.Arrays;
+import java.util.HashSet;
+import java.util.Set;
+
+@ParameterizedClass
+@MethodSource("redis.clients.jedis.commands.CommandsTestsParameters#respVersions")
+public class SentinelAllKindOfValuesCommandsIT extends AllKindOfValuesCommandsTestBase {
+
+  static final HostAndPort sentinel1 = HostAndPorts.getSentinelServers().get(1);
+
+  static final HostAndPort sentinel2 = HostAndPorts.getSentinelServers().get(3);
+
+  static final Set<HostAndPort> sentinels = new HashSet<>(Arrays.asList(sentinel1, sentinel2));
+
+  static final JedisClientConfig sentinelClientConfig = DefaultJedisClientConfig.builder().build();
+
+  static final EndpointConfig primary = HostAndPorts.getRedisEndpoint("standalone2-primary");
+
+  @RegisterExtension
+  public RedisVersionCondition versionCondition = new RedisVersionCondition(
+      primary.getHostAndPort(), primary.getClientConfigBuilder().build());
+
+  @RegisterExtension
+  public EnabledOnCommandCondition enabledOnCommandCondition = new EnabledOnCommandCondition(
+      primary.getHostAndPort(), primary.getClientConfigBuilder().build());
+
+  public SentinelAllKindOfValuesCommandsIT(RedisProtocol protocol) {
+    super(protocol);
+  }
+
+  @Override
+  protected UnifiedJedis createTestClient() {
+
+    return JedisSentineled.builder()
+        .clientConfig(primary.getClientConfigBuilder().protocol(protocol).build())
+        .sentinels(sentinels).sentinelClientConfig(sentinelClientConfig).masterName("mymaster")
+        .build();
+  }
+
+}