svn commit: r1559844 - /lucene/dev/trunk/solr/solrj/src/java/org/apache/solr/common/cloud/ConnectionManager.java

[ma...@apache.org]

Author: markrmiller
Date: Mon Jan 20 22:25:44 2014
New Revision: 1559844

URL: http://svn.apache.org/r1559844
Log:
SOLR-5577: Protect against NPE race with close - spotted by Greg Chanan

Modified:
    lucene/dev/trunk/solr/solrj/src/java/org/apache/solr/common/cloud/ConnectionManager.java

Modified: lucene/dev/trunk/solr/solrj/src/java/org/apache/solr/common/cloud/ConnectionManager.java
URL: http://svn.apache.org/viewvc/lucene/dev/trunk/solr/solrj/src/java/org/apache/solr/common/cloud/ConnectionManager.java?rev=1559844&r1=1559843&r2=1559844&view=diff
==============================================================================
--- lucene/dev/trunk/solr/solrj/src/java/org/apache/solr/common/cloud/ConnectionManager.java (original)
+++ lucene/dev/trunk/solr/solrj/src/java/org/apache/solr/common/cloud/ConnectionManager.java Mon Jan 20 22:25:44 2014
@@ -63,20 +63,14 @@ public class ConnectionManager implement
   }
   
   private synchronized void connected() {
-    if (disconnectedTimer != null) {
-      disconnectedTimer.cancel();
-      disconnectedTimer = null;
-    }
+    cancelTimer();
     connected = true;
     likelyExpired = false;
     notifyAll();
   }
 
   private synchronized void disconnected() {
-    if (disconnectedTimer != null) {
-      disconnectedTimer.cancel();
-      disconnectedTimer = null;
-    }
+    cancelTimer();
     if (!isClosed) {
       disconnectedTimer = new Timer(true);
       disconnectedTimer.schedule(new TimerTask() {
@@ -90,14 +84,23 @@ public class ConnectionManager implement
       if (isClosed) {
         // we might have closed after getting by isClosed
         // and before starting the new timer
-        disconnectedTimer.cancel();
-        disconnectedTimer = null;
+        cancelTimer();
       }
     }
     connected = false;
     notifyAll();
   }
 
+  private void cancelTimer() {
+    try {
+      this.disconnectedTimer.cancel();
+    } catch (NullPointerException e) {
+      // fine
+    } finally {
+      this.disconnectedTimer = null;
+    }
+  }
+
   @Override
   public void process(WatchedEvent event) {
     if (log.isInfoEnabled()) {
@@ -117,10 +120,10 @@ public class ConnectionManager implement
       clientConnected.countDown();
       connectionStrategy.connected();
     } else if (state == KeeperState.Expired) {
-      if (disconnectedTimer != null) {
-        disconnectedTimer.cancel();
-        disconnectedTimer = null;
-      }
+      // we don't call disconnected because there
+      // is no need to start the timer - if we are expired
+      // likelyExpired can just be set to true
+      cancelTimer();
       
       connected = false;
       likelyExpired = true;
@@ -197,13 +200,7 @@ public class ConnectionManager implement
   public void close() {
     this.isClosed = true;
     this.likelyExpired = true;
-    try {
-      this.disconnectedTimer.cancel();
-    } catch (NullPointerException e) {
-      // fine
-    } finally {
-      this.disconnectedTimer = null;
-    }
+    cancelTimer();
   }
   
   public boolean isLikelyExpired() {