From 75d2ba751ba59c1cf5e13ddad84d4f0d3a82fb3e Mon Sep 17 00:00:00 2001
From: Nelson Rodrigues <nelson@nelsonjrodrigues.com>
Date: Fri, 25 Jul 2014 18:06:33 -0700
Subject: [PATCH] Race condition when switching masters in JedisSentinelPool

Instead of recreating GenericObjectPool, we change the
underlying factory destination host. When returning
objects to the pool we make sure they are pointing at
the correct master.
---
 .../redis/clients/jedis/JedisFactory.java     | 23 ++++++++++++++-----
 .../clients/jedis/JedisSentinelPool.java      | 12 +++++++---
 2 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/src/main/java/redis/clients/jedis/JedisFactory.java b/src/main/java/redis/clients/jedis/JedisFactory.java
index 3597d69..c04b43d 100644
--- a/src/main/java/redis/clients/jedis/JedisFactory.java
+++ b/src/main/java/redis/clients/jedis/JedisFactory.java
@@ -1,5 +1,7 @@
 package redis.clients.jedis;
 
+import java.util.concurrent.atomic.AtomicReference;
+
 import org.apache.commons.pool2.PooledObject;
 import org.apache.commons.pool2.PooledObjectFactory;
 import org.apache.commons.pool2.impl.DefaultPooledObject;
@@ -8,8 +10,7 @@ import org.apache.commons.pool2.impl.DefaultPooledObject;
  * PoolableObjectFactory custom impl.
  */
 class JedisFactory implements PooledObjectFactory<Jedis> {
-    private final String host;
-    private final int port;
+    private final AtomicReference<HostAndPort> hostAndPort = new AtomicReference<HostAndPort>();
     private final int timeout;
     private final String password;
     private final int database;
@@ -23,14 +24,17 @@ class JedisFactory implements PooledObjectFactory<Jedis> {
     public JedisFactory(final String host, final int port, final int timeout,
 	    final String password, final int database, final String clientName) {
 	super();
-	this.host = host;
-	this.port = port;
+	this.hostAndPort.set(new HostAndPort(host, port));
 	this.timeout = timeout;
 	this.password = password;
 	this.database = database;
 	this.clientName = clientName;
     }
 
+    public void setHostAndPort(final HostAndPort hostAndPort) {
+        this.hostAndPort.set(hostAndPort);
+    }
+
     @Override
     public void activateObject(PooledObject<Jedis> pooledJedis)
 	    throws Exception {
@@ -60,7 +64,8 @@ class JedisFactory implements PooledObjectFactory<Jedis> {
 
     @Override
     public PooledObject<Jedis> makeObject() throws Exception {
-	final Jedis jedis = new Jedis(this.host, this.port, this.timeout);
+    final HostAndPort hostAndPort = this.hostAndPort.get();
+	final Jedis jedis = new Jedis(hostAndPort.getHost(), hostAndPort.getPort(), this.timeout);
 
 	jedis.connect();
 	if (null != this.password) {
@@ -86,7 +91,13 @@ class JedisFactory implements PooledObjectFactory<Jedis> {
     public boolean validateObject(PooledObject<Jedis> pooledJedis) {
 	final BinaryJedis jedis = pooledJedis.getObject();
 	try {
-	    return jedis.isConnected() && jedis.ping().equals("PONG");
+	    HostAndPort hostAndPort = this.hostAndPort.get();
+
+	    String connectionHost = jedis.getClient().getHost();
+	    int connectionPort = jedis.getClient().getPort();
+
+	    return hostAndPort.getHost().equals(connectionHost) && hostAndPort.getPort() == connectionPort &&
+	            jedis.isConnected() && jedis.ping().equals("PONG");
 	} catch (final Exception e) {
 	    return false;
 	}
diff --git a/src/main/java/redis/clients/jedis/JedisSentinelPool.java b/src/main/java/redis/clients/jedis/JedisSentinelPool.java
index 0ff0dff..2e076b1 100644
--- a/src/main/java/redis/clients/jedis/JedisSentinelPool.java
+++ b/src/main/java/redis/clients/jedis/JedisSentinelPool.java
@@ -74,6 +74,7 @@ public class JedisSentinelPool extends Pool<Jedis> {
 	initPool(master);
     }
 
+    private volatile JedisFactory factory;
     private volatile HostAndPort currentHostMaster;
 
     public void destroy() {
@@ -91,10 +92,15 @@ public class JedisSentinelPool extends Pool<Jedis> {
     private void initPool(HostAndPort master) {
 	if (!master.equals(currentHostMaster)) {
 	    currentHostMaster = master;
+	    if (factory == null) {
+	        factory = new JedisFactory(master.getHost(), master.getPort(),
+	                                   timeout, password, database);
+	        initPool(poolConfig, factory);
+	    } else {
+	        factory.setHostAndPort(currentHostMaster);
+	    }
+
 	    log.info("Created JedisPool to master at " + master);
-	    initPool(poolConfig,
-		    new JedisFactory(master.getHost(), master.getPort(),
-			    timeout, password, database));
 	}
     }