Fixes #791 Exceptions in close cause empty JedisPool and then deadlock

This commit is contained in:
Tobias Ara Svensson
2014-11-24 12:23:42 +01:00
parent 0c9c4c8b79
commit 3257827f01
2 changed files with 67 additions and 4 deletions

View File

@@ -5,6 +5,7 @@ import java.net.URI;
import org.apache.commons.pool2.impl.GenericObjectPool;
import org.apache.commons.pool2.impl.GenericObjectPoolConfig;
import redis.clients.jedis.exceptions.JedisException;
import redis.clients.util.JedisURIHelper;
import redis.clients.util.Pool;
@@ -112,10 +113,17 @@ public class JedisPool extends Pool<Jedis> {
}
public void returnResource(final Jedis resource) {
if (resource != null) {
resource.resetState();
returnResourceObject(resource);
}
if (resource != null) {
try {
resource.resetState();
returnResourceObject(resource);
}
catch (Exception e) {
returnBrokenResource(resource);
throw new JedisException(
"Could not return the resource to the pool", e);
}
}
}
public int getNumActive() {

View File

@@ -1,8 +1,13 @@
package redis.clients.jedis.tests;
import java.net.SocketTimeoutException;
import java.net.URI;
import java.net.URISyntaxException;
import java.util.concurrent.atomic.AtomicInteger;
import org.apache.commons.pool2.PooledObject;
import org.apache.commons.pool2.PooledObjectFactory;
import org.apache.commons.pool2.impl.DefaultPooledObject;
import org.apache.commons.pool2.impl.GenericObjectPoolConfig;
import org.junit.Assert;
import org.junit.Test;
@@ -206,6 +211,56 @@ public class JedisPoolTest extends Assert {
assertTrue(pool0.isClosed());
}
@Test
public void returnResourceDestroysResourceOnException() {
class CrashingJedis extends Jedis {
@Override
public void resetState() {
throw new RuntimeException();
}
}
final AtomicInteger destroyed = new AtomicInteger(0);
class CrashingJedisPooledObjectFactory implements PooledObjectFactory<Jedis> {
@Override
public PooledObject<Jedis> makeObject() throws Exception {
return new DefaultPooledObject<Jedis>(new CrashingJedis());
}
@Override
public void destroyObject(PooledObject<Jedis> p) throws Exception {
destroyed.incrementAndGet();
}
@Override
public boolean validateObject(PooledObject<Jedis> p) {
return true;
}
@Override
public void activateObject(PooledObject<Jedis> p) throws Exception {}
@Override
public void passivateObject(PooledObject<Jedis> p) throws Exception {}
}
GenericObjectPoolConfig config = new GenericObjectPoolConfig();
config.setMaxTotal(1);
JedisPool pool = new JedisPool(config, hnp.getHost(), hnp.getPort(), 2000, "foobared");
pool.initPool(config, new CrashingJedisPooledObjectFactory());
Jedis crashingJedis = pool.getResource();
try {
pool.returnResource(crashingJedis);
}
catch (Exception ignored) {}
assertEquals(destroyed.get(), 1);
}
@Test
public void returnResourceShouldResetState() {
GenericObjectPoolConfig config = new GenericObjectPoolConfig();