ZOOKEEPER-370. Fix critical problems reported by findbugs. (flavio via mahadev)

git-svn-id: https://svn.apache.org/repos/asf/hadoop/zookeeper/trunk@763395 13f79535-47bb-0310-9956-ffa450edef68

CHANGES.txt

@@ -41,6 +41,8 @@ BUGFIXES:
 
   ZOOKEEPER-363. NPE when recovering ledger with no hint. (flavio via mahadev)
 
+  ZOOKEEPER-370. Fix critical problems reported by findbugs. (flavio via mahadev)
+
 IMPROVEMENTS:
   ZOOKEEPER-308. improve the atomic broadcast performance 3x.
   (breed via mahadev)

src/java/main/org/apache/zookeeper/server/quorum/LeaderElection.java

@@ -39,7 +39,8 @@ import org.apache.zookeeper.server.quorum.QuorumPeer.ServerState;
 
 public class LeaderElection implements Election  {
     private static final Logger LOG = Logger.getLogger(LeaderElection.class);
-
+    private static Random epochGen = new Random();
+    
     QuorumPeer self;
 
     public LeaderElection(QuorumPeer self) {
@@ -130,7 +131,7 @@ public class LeaderElection implements Election  {
                 responseBytes.length);
         HashMap<InetSocketAddress, Vote> votes = new HashMap<InetSocketAddress, Vote>(
                 self.quorumPeers.size());
-        int xid = new Random().nextInt();
+        int xid = epochGen.nextInt();
         while (self.running) {
             votes.clear();
             requestBuffer.clear();

src/java/main/org/apache/zookeeper/server/quorum/QuorumCnxManager.java

@@ -72,12 +72,7 @@ public class QuorumCnxManager {
      * Packet size
      */
     int packetSize;
-
-    /*
-     * Port to listen on
-     */
-    int port;
-
+    
     /*
      * Challenge to initiate connections
      */
@@ -126,27 +121,11 @@ public class QuorumCnxManager {
         this.senderWorkerMap = new ConcurrentHashMap<Long, SendWorker>();
         this.self = self;
 
-        // Generates a challenge to guarantee one connection between pairs of
-        // servers
-        //genChallenge();
-
         // Starts listener thread that waits for connection requests 
         listener = new Listener();
         listener.start();
     }
 
-    void genChallenge() {
-        try{
-            Random rand = new Random(System.currentTimeMillis()
-                + InetAddress.getLocalHost().hashCode());
-            long newValue = rand.nextLong();
-            challenge = newValue;
-        } catch(UnknownHostException e){
-            LOG.error("Cannot resolve local address");
-            challenge = 0;
-        }
-    }
-
     /**
      * If this server has initiated the connection, then it gives up on the
      * connection if it loses challenge. Otherwise, it keeps the connection.
@@ -177,30 +156,25 @@ public class QuorumCnxManager {
             }
         // Otherwise proceed with the connection
         } else {
-                if (s != null) {
-                    SendWorker sw = new SendWorker(s, sid);
-                    RecvWorker rw = new RecvWorker(s, sid);
-                    sw.setRecv(rw);
-
-                    if (senderWorkerMap
-                            .containsKey(sid)) {
-                        senderWorkerMap.get(sid).finish();
-                    }
-
-                    if (!queueSendMap.containsKey(sid)) {
-                        queueSendMap.put(sid, new ArrayBlockingQueue<ByteBuffer>(
-                                CAPACITY));
-                    }
+        	SendWorker sw = new SendWorker(s, sid);
+        	RecvWorker rw = new RecvWorker(s, sid);
+        	sw.setRecv(rw);
+        	
+        	if (senderWorkerMap
+        			.containsKey(sid)) {
+        		senderWorkerMap.get(sid).finish();
+        	}
+
+        	if (!queueSendMap.containsKey(sid)) {
+        		queueSendMap.put(sid, new ArrayBlockingQueue<ByteBuffer>(
+        				CAPACITY));
+        	}
                     
-                    senderWorkerMap.put(sid, sw);
-                    sw.start();
-                    rw.start();
+        	senderWorkerMap.put(sid, sw);
+        	sw.start();
+        	rw.start();
 
-                    return true;
-                } else {
-                    LOG.warn("Channel null");
-                    return false;
-                }
+        	return true;
             
         }
         return false;
@@ -247,30 +221,24 @@ public class QuorumCnxManager {
             }
         //Otherwise start worker threads to receive data.
         } else {
-            
-                if (s != null) {
-                    SendWorker sw = new SendWorker(s, sid);
-                    RecvWorker rw = new RecvWorker(s, sid);
-                    sw.setRecv(rw);
+        	SendWorker sw = new SendWorker(s, sid);
+        	RecvWorker rw = new RecvWorker(s, sid);
+        	sw.setRecv(rw);
 
-                    if (senderWorkerMap.containsKey(sid)) {
-                        senderWorkerMap.get(sid).finish();
-                    }
+        	if (senderWorkerMap.containsKey(sid)) {
+        		senderWorkerMap.get(sid).finish();
+        	}
                     
-                    senderWorkerMap.put(sid, sw);
+        	senderWorkerMap.put(sid, sw);
                     
-                    if (!queueSendMap.containsKey(sid)) {
-                        queueSendMap.put(sid, new ArrayBlockingQueue<ByteBuffer>(
-                                CAPACITY));
-                    }      
-                    sw.start();
-                    rw.start();
-
-                    return true;
-                } else {
-                    LOG.warn("Channel null");
-                    return false;
-                }
+        	if (!queueSendMap.containsKey(sid)) {
+        		queueSendMap.put(sid, new ArrayBlockingQueue<ByteBuffer>(
+        				CAPACITY));
+        	}      
+        	sw.start();
+        	rw.start();
+
+        	return true;    
         }
         return false;
     }