Closing a broken connection throws an exception if reconnection fails

[sa7msw]

Given a broken Jedis client connection, if you try to close() it and its connection pool replacement can't connect to the server, a JedisException propagates out from the close() method. This seems both undesirable and unexpected - as a user I just want to close and release any resources allocated to the connection, I should not have to deal with problems caused by other side effects like repopulating the connection pool.

redis.clients.jedis.exceptions.JedisException: Could not return the broken resource to the pool
        at redis.clients.jedis.util.Pool.returnBrokenResource(Pool.java:64)
        at redis.clients.jedis.Jedis.close(Jedis.java:365)
        at ...
Caused by: redis.clients.jedis.exceptions.JedisConnectionException: Failed to connect to 172.20.1.10:6379.
        at redis.clients.jedis.DefaultJedisSocketFactory.connectToFirstSuccessfulHost(DefaultJedisSocketFactory.java:68)
        at redis.clients.jedis.DefaultJedisSocketFactory.createSocket(DefaultJedisSocketFactory.java:94)
        at com.opencloud.resource.memdb.storage.redis.jedis.MetricsJedisSocketFactory.createSocket(0.0.1-SNAPSHOT:35)
        at redis.clients.jedis.Connection.connect(Connection.java:269)
        at redis.clients.jedis.Connection.initializeFromClientConfig(Connection.java:507)
        at redis.clients.jedis.Connection.<init>(Connection.java:109)
        at redis.clients.jedis.Jedis.<init>(Jedis.java:290)
        at redis.clients.jedis.JedisFactory.makeObject(JedisFactory.java:180)
        at org.apache.commons.pool2.impl.GenericObjectPool.create(GenericObjectPool.java:555)
        at org.apache.commons.pool2.impl.GenericObjectPool.addObject(GenericObjectPool.java:219)
        at org.apache.commons.pool2.impl.GenericObjectPool.invalidateObject(GenericObjectPool.java:939)
        at org.apache.commons.pool2.impl.GenericObjectPool.invalidateObject(GenericObjectPool.java:909)
        at redis.clients.jedis.util.Pool.returnBrokenResource(Pool.java:62)
        ... 8 more
        Suppressed: java.net.SocketTimeoutException: connect timed out
                at java.base/java.net.PlainSocketImpl.socketConnect(Native Method)
                at java.base/java.net.AbstractPlainSocketImpl.doConnect(AbstractPlainSocketImpl.java:412)
                at java.base/java.net.AbstractPlainSocketImpl.connectToAddress(AbstractPlainSocketImpl.java:255)
                at java.base/java.net.AbstractPlainSocketImpl.connect(AbstractPlainSocketImpl.java:237)
                at java.base/java.net.SocksSocketImpl.connect(SocksSocketImpl.java:392)
                at java.base/java.net.Socket.connect(Socket.java:609)
                at redis.clients.jedis.DefaultJedisSocketFactory.connectToFirstSuccessfulHost(DefaultJedisSocketFactory.java:80)
                ... 20 more

[ggivo]

@sa7msw
Could you provide the Jedis version in use?

[sa7msw]

Originally I found the issue with 7.5.0 but I've written a repro class and tested it with 8.0.0 as well and it behaves exactly the same. Here's my repro test class:

import org.apache.commons.pool2.impl.GenericObjectPoolConfig;
import redis.clients.jedis.Jedis;
import redis.clients.jedis.JedisPool;

public class Test {
    public static void main(String[] args) throws Exception {
        if (args.length != 2) {
            System.out.println("Usage: java Test <host> <port>");
            System.exit(1);
        }

        System.out.println("Connecting to Redis server at " + args[0] + ":" + args[1]);
        GenericObjectPoolConfig<Jedis> poolConfig = new GenericObjectPoolConfig<>();
        poolConfig.setMaxTotal(5);
        poolConfig.setMinIdle(2);
        JedisPool jedisPool = new JedisPool(poolConfig, args[0], Integer.parseInt(args[1]));
        Jedis jedis = jedisPool.getResource();
        jedis.ping();
        System.out.println("Successfully connected to Redis");

        System.out.println("\nNow stop the Redis server and press Enter to continue...");
        System.in.read();

        try {
            System.out.println("Attempting to ping Redis server after it has been stopped");
            jedis.ping();
            System.out.println("Unexpectedly succeeded in pinging Redis server after it has been stopped, aborting test");
            System.exit(2);
        }
        catch (RuntimeException e) {
            System.out.println("Caught exception during ping as expected: " + e.getMessage());
        }

        System.out.println("Closing broken connection");
        jedis.close();
    }
}

Example output:

Connecting to Redis server at localhost:6379
Successfully connected to Redis

Now stop the Redis server and press Enter to continue...

Attempting to ping Redis server after it has been stopped
Caught exception during ping as expected: Unexpected end of stream.
Closing broken connection
Exception in thread "main" redis.clients.jedis.exceptions.JedisException: Could not return the broken resource to the pool
	at redis.clients.jedis.util.Pool.returnBrokenResource(Pool.java:64)
	at redis.clients.jedis.Jedis.close(Jedis.java:396)
	at Test.main(Test.java:35)
Caused by: redis.clients.jedis.exceptions.JedisConnectionException: Failed to connect to localhost:6379.
	at redis.clients.jedis.DefaultJedisSocketFactory.connectToFirstSuccessfulHost(DefaultJedisSocketFactory.java:68)
	at redis.clients.jedis.DefaultJedisSocketFactory.createSocket(DefaultJedisSocketFactory.java:94)
	at redis.clients.jedis.Connection.connect(Connection.java:396)
	at redis.clients.jedis.Connection.initializeFromClientConfig(Connection.java:685)
	at redis.clients.jedis.Connection.<init>(Connection.java:171)
	at redis.clients.jedis.Jedis.<init>(Jedis.java:321)
	at redis.clients.jedis.JedisFactory.makeObject(JedisFactory.java:206)
	at org.apache.commons.pool2.impl.GenericObjectPool.create(GenericObjectPool.java:555)
	at org.apache.commons.pool2.impl.GenericObjectPool.addObject(GenericObjectPool.java:219)
	at org.apache.commons.pool2.impl.GenericObjectPool.invalidateObject(GenericObjectPool.java:939)
	at org.apache.commons.pool2.impl.GenericObjectPool.invalidateObject(GenericObjectPool.java:909)
	at redis.clients.jedis.util.Pool.returnBrokenResource(Pool.java:62)
	... 2 more
	Suppressed: java.net.ConnectException: Connection refused (Connection refused)
		at java.base/java.net.PlainSocketImpl.socketConnect(Native Method)
		at java.base/java.net.AbstractPlainSocketImpl.doConnect(AbstractPlainSocketImpl.java:412)
		at java.base/java.net.AbstractPlainSocketImpl.connectToAddress(AbstractPlainSocketImpl.java:255)
		at java.base/java.net.AbstractPlainSocketImpl.connect(AbstractPlainSocketImpl.java:237)
		at java.base/java.net.SocksSocketImpl.connect(SocksSocketImpl.java:392)
		at java.base/java.net.Socket.connect(Socket.java:609)
		at redis.clients.jedis.DefaultJedisSocketFactory.connectToFirstSuccessfulHost(DefaultJedisSocketFactory.java:80)
		... 13 more

[ggivo]

Confirmed — and thanks for the reproducer and stack trace, they made this straightforward to pin down.

This is a commons-pool2 2.13.x behaviour change (POOL-431): invalidateObject() now unconditionally calls addObject(), so when the server is down the replacement connect fails and the exception escapes Jedis#close().

Jedis ships commons-pool2 2.13.1 since 8.0.0 (#4409); Jedis 7.x declares 2.12.1.

I reproduced your scenario across a version matrix: any Jedis version with commons-pool2 2.13.1 on the classpath throws on close(); with 2.12.1 it is silent. Seeing this on Jedis 7.5.0 therefore means your build resolved commons-pool2 2.13.x transitively — your stack trace (invalidateObject → addObject) matches 2.13.x line numbers, not the 2.12.1 code Jedis 7.5.0 declares.

We agree close() should not throw for a failed pool replacement. A fix is proposed upstream in apache/commons-pool#454 (waiter-aware, non-blocking replenishment that swallows factory exceptions), but it is still awaiting review and no released commons-pool2 version contains it.

Keeping this open to track a Jedis-side mitigation while the upstream fix is pending.

Note: the exception path is not entirely new — with 2.12.1 the same JedisException escapes close() when another thread is blocked in getResource() at that moment; 2.13.x just made it unconditional.