ZOOKEEPER-3223: Configure Spotbugs - on branch 3.4

- add spotbugs configuration (default)
- make build pass spotbugs

Fix errors:
[ERROR] Increment of volatile field org.apache.zookeeper.server.quorum.AuthFastLeaderElection.logicalclock in org.apache.zookeeper.server.quorum.AuthFastLeaderElection.leaveInstance() [org.apache.zookeeper.server.quorum.AuthFastLeaderElection] At AuthFastLeaderElection.java:[line 780] VO_VOLATILE_INCREMENT
[ERROR] Increment of volatile field org.apache.zookeeper.server.quorum.AuthFastLeaderElection.logicalclock in org.apache.zookeeper.server.quorum.AuthFastLeaderElection.lookForLeader() [org.apache.zookeeper.server.quorum.AuthFastLeaderElection] At AuthFastLeaderElection.java:[line 854] VO_VOLATILE_INCREMENT
[ERROR] Switch statement found in org.apache.zookeeper.server.quorum.AuthFastLeaderElection$Messenger$WorkerReceiver.run() where default case is missing [org.apache.zookeeper.server.quorum.AuthFastLeaderElection$Messenger$WorkerReceiver] At AuthFastLeaderElection.java:[lines 270-278] SF_SWITCH_NO_DEFAULT
[ERROR] Switch statement found in org.apache.zookeeper.server.quorum.AuthFastLeaderElection$Messenger$WorkerSender.process(AuthFastLeaderElection$ToSend) where default case is missing [org.apache.zookeeper.server.quorum.AuthFastLeaderElection$Messenger$WorkerSender] At AuthFastLeaderElection.java:[lines 468-700] SF_SWITCH_NO_DEFAULT
[ERROR] Unread field: org.apache.zookeeper.server.quorum.LearnerSessionTracker.expirer [org.apache.zookeeper.server.quorum.LearnerSessionTracker] At LearnerSessionTracker.java:[line 45] URF_UNREAD_FIELD
[ERROR] org.apache.zookeeper.server.quorum.QuorumPeer.writeLongToFile(String, long) may fail to close stream [org.apache.zookeeper.server.quorum.QuorumPeer] At QuorumPeer.java:[line 1387] OS_OPEN_STREAM
[ERROR] Inconsistent synchronization of org.apache.zookeeper.server.upgrade.DataNodeV1.data; locked 50% of time [org.apache.zookeeper.server.upgrade.DataNodeV1, org.apache.zookeeper.server.upgrade.DataNodeV1, org.apache.zookeeper.server.upgrade.UpgradeSnapShotV1, org.apache.zookeeper.server.upgrade.DataNodeV1, org.apache.zookeeper.server.upgrade.DataTreeV1, org.apache.zookeeper.server.upgrade.DataTreeV1] Unsynchronized access at DataNodeV1.java:[line 98]Unsynchronized access at DataNodeV1.java:[line 91]Unsynchronized access at UpgradeSnapShotV1.java:[line 270]Synchronized access at DataNodeV1.java:[line 118]Synchronized access at DataTreeV1.java:[line 263]Synchronized access at DataTreeV1.java:[line 283] IS2_INCONSISTENT_SYNC
[ERROR] Dead store to $L9 in org.apache.zookeeper.server.upgrade.DataTreeV1.processTxn(TxnHeader, Record) [org.apache.zookeeper.server.upgrade.DataTreeV1] At DataTreeV1.java:[line 417] DLS_DEAD_LOCAL_STORE
[ERROR] Switch statement found in org.apache.zookeeper.server.upgrade.DataTreeV1.processTxn(TxnHeader, Record) where default case is missing [org.apache.zookeeper.server.upgrade.DataTreeV1] At DataTreeV1.java:[lines 392-417] SF_SWITCH_NO_DEFAULT
[ERROR] org.apache.zookeeper.server.upgrade.DataTreeV1.dumpEphemerals() makes inefficient use of keySet iterator instead of entrySet iterator [org.apache.zookeeper.server.upgrade.DataTreeV1] At DataTreeV1.java:[line 545] WMI_WRONG_MAP_ITERATOR

[ERROR] Inconsistent synchronization of org.apache.zookeeper.recipes.leader.LeaderElectionSupport.hostName; locked 60% of time [org.apache.zookeeper.recipes.leader.LeaderElectionSupport, org.apache.zookeeper.recipes.leader.LeaderElectionSupport, org.apache.zookeeper.recipes.leader.LeaderElectionSupport, org.apache.zookeeper.recipes.leader.LeaderElectionSupport, org.apache.zookeeper.recipes.leader.LeaderElectionSupport, org.apache.zookeeper.recipes.leader.LeaderElectionSupport] Unsynchronized access at LeaderElectionSupport.java:[line 441]Unsynchronized access at LeaderElectionSupport.java:[line 445]Synchronized access at LeaderElectionSupport.java:[line 185]Synchronized access at LeaderElectionSupport.java:[line 186]Synchronized access at LeaderElectionSupport.java:[line 138]Synchronized access at LeaderElectionSupport.java:[line 387] IS2_INCONSISTENT_SYNC
[ERROR] Inconsistent synchronization of org.apache.zookeeper.recipes.leader.LeaderElectionSupport.leaderOffer; locked 53% of time [org.apache.zookeeper.recipes.leader.LeaderElectionSupport, org.apache.zookeeper.recipes.leader.LeaderElectionSupport, org.apache.zookeeper.recipes.leader.LeaderElectionSupport, org.apache.zookeeper.recipes.leader.LeaderElectionSupport, org.apache.zookeeper.recipes.leader.LeaderElectionSupport, org.apache.zookeeper.recipes.leader.LeaderElectionSupport, org.apache.zookeeper.recipes.leader.LeaderElectionSupport, org.apache.zookeeper.recipes.leader.LeaderElectionSupport, org.apache.zookeeper.recipes.leader.LeaderElectionSupport, org.apache.zookeeper.recipes.leader.LeaderElectionSupport, org.apache.zookeeper.recipes.leader.LeaderElectionSupport, org.apache.zookeeper.recipes.leader.LeaderElectionSupport, org.apache.zookeeper.recipes.leader.LeaderElectionSupport, org.apache.zookeeper.recipes.leader.LeaderElectionSupport] Unsynchronized access at LeaderElectionSupport.java:[line 201]Unsynchronized access at LeaderElectionSupport.java:[line 203]Unsynchronized access at LeaderElectionSupport.java:[line 218]Unsynchronized access at LeaderElectionSupport.java:[line 339]Unsynchronized access at LeaderElectionSupport.java:[line 273]Unsynchronized access at LeaderElectionSupport.java:[line 240]Synchronized access at LeaderElectionSupport.java:[line 165]Synchronized access at LeaderElectionSupport.java:[line 167]Synchronized access at LeaderElectionSupport.java:[line 168]Synchronized access at LeaderElectionSupport.java:[line 183]Synchronized access at LeaderElectionSupport.java:[line 185]Synchronized access at LeaderElectionSupport.java:[line 186]Synchronized access at LeaderElectionSupport.java:[line 190]Synchronized access at LeaderElectionSupport.java:[line 387] IS2_INCONSISTENT_SYNC
[ERROR] org.apache.zookeeper.recipes.leader.LeaderOffer$IdComparator implements Comparator but not Serializable [org.apache.zookeeper.recipes.leader.LeaderOffer$IdComparator] At LeaderOffer.java:[lines 75-79] SE_COMPARATOR_SHOULD_BE_SERIALIZABLE
[INFO]

Author: Enrico Olivelli <eolivelli@apache.org>

Reviewers: andor@apache.org

Closes #764 from eolivelli/fix/ZOOKEEPER-3223-branch-3.4 and squashes the following commits:

488925dc7 [Enrico Olivelli] Upgrade to spotbugs 3.1.9
66f43cdbc [Enrico Olivelli] Suppress IS2_INCONSISTENT_SYNC on DataNodeV1#data
716e26984 [Enrico Olivelli] revert synchronized blocks on core code
13415fb84 [Enrico Olivelli] Remove unused imports
8079261e0 [Enrico Olivelli] Fix recipes and disable spotbugs on contrib
b6dac0519 [Enrico Olivelli] Fix spotbugs errors
febc57bc4 [Enrico Olivelli] ZOOKEEPER-3223: Configure Spotbugs

build.xml

@@ -27,6 +27,7 @@ xmlns:cs="antlib:com.puppycrawl.tools.checkstyle">
     <!-- Dependency versions                                    -->
     <!-- ====================================================== -->
     <property name="slf4j.version" value="1.7.25"/>
+    <property name="spotbugsannotations.version" value="3.1.9"/>
 
     <property name="wagon-http.version" value="2.4"/>
     <property name="maven-ant-tasks.version" value="2.1.3"/>

excludeFindBugsFilter.xml

@@ -0,0 +1,14 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<FindBugsFilter>
+    <!-- this work work on JDK11  https://github.com/spotbugs/spotbugs-maven-plugin/issues/92 -->
+    <Bug pattern="RCN_REDUNDANT_NULLCHECK_WOULD_HAVE_BEEN_A_NPE"/>
+
+    <!-- this problem is to be addressed in ZOOKEEPER-3227 -->
+    <Bug pattern="DM_DEFAULT_ENCODING"/>
+
+    <!-- not really a problem -->
+    <Bug pattern="DM_EXIT"/>
+
+</FindBugsFilter>
+

ivy.xml

@@ -44,7 +44,7 @@
   <dependencies>
     <dependency org="org.slf4j" name="slf4j-api" rev="${slf4j.version}"/>
     <dependency org="org.slf4j" name="slf4j-log4j12" rev="${slf4j.version}" transitive="false"/>
-
+    <dependency org="com.github.spotbugs" name="spotbugs-annotations" rev="${spotbugsannotations.version}" />
     <dependency org="org.apache.maven.wagon" name="wagon-http" rev="${wagon-http.version}"
                 conf="mvn-ant-task->default"/>
     <dependency org="org.apache.maven" name="maven-ant-tasks" rev="${maven-ant-tasks.version}"

pom.xml

@@ -281,6 +281,7 @@
     <commons-lang.version>2.4</commons-lang.version>
     <apache-directory-server.version>2.0.0-M15</apache-directory-server.version>
     <apache-directory-api.version>1.0.0-M20</apache-directory-api.version>
+    <spotbugsannotations.version>3.1.9</spotbugsannotations.version>
   </properties>
 
   <dependencyManagement>
@@ -654,6 +655,13 @@
           </exclusion>
         </exclusions>
       </dependency>
+      <dependency>
+         <groupId>com.github.spotbugs</groupId>
+         <artifactId>spotbugs-annotations</artifactId>
+         <version>${spotbugsannotations.version}</version>
+         <scope>provided</scope>
+         <optional>true</optional>
+        </dependency>
     </dependencies>
   </dependencyManagement>
 
@@ -710,6 +718,14 @@
           <artifactId>maven-install-plugin</artifactId>
           <version>3.0.0-M1</version>
         </plugin>
+        <plugin>
+          <groupId>com.github.spotbugs</groupId>
+          <artifactId>spotbugs-maven-plugin</artifactId>
+          <version>3.1.9</version>
+          <configuration>
+            <excludeFilterFile>excludeFindBugsFilter.xml</excludeFilterFile>
+          </configuration>
+        </plugin>
       </plugins>
     </pluginManagement>
 
@@ -737,7 +753,11 @@
           </execution>
         </executions>
       </plugin>
-    </plugins>
+      <plugin>
+        <groupId>com.github.spotbugs</groupId>
+        <artifactId>spotbugs-maven-plugin</artifactId>
+      </plugin>
+      </plugins>
   </build>
 
   <reporting>

zookeeper-contrib/pom.xml

@@ -34,11 +34,25 @@
   <description>
     Contrib projects to Apache ZooKeeper
   </description>
-
+  <build>
+    <pluginManagement>
+       <plugins>
+          <plugin>
+            <groupId>com.github.spotbugs</groupId>
+            <artifactId>spotbugs-maven-plugin</artifactId>
+            <version>3.1.9</version>
+            <configuration>
+                <!-- No spotbugs for contri modules -->
+                <skip>true</skip>
+            </configuration>
+          </plugin>
+        </plugins>
+      </pluginManagement>
+  </build>
   <modules>
     <module>zookeeper-contrib-loggraph</module>
     <module>zookeeper-contrib-rest</module>
     <module>zookeeper-contrib-zooinspector</module>
   </modules>
 
-</project>
+</project>

zookeeper-jute/pom.xml

@@ -145,6 +145,14 @@
           </execution>
         </executions>
       </plugin>
+      <plugin>
+        <!-- spotbugs does not make sense for generated code -->
+        <groupId>com.github.spotbugs</groupId>
+        <artifactId>spotbugs-maven-plugin</artifactId>
+        <configuration>
+            <skip>true</skip>
+        </configuration>
+      </plugin>
     </plugins>
   </build>
 

zookeeper-recipes/zookeeper-recipes-election/src/main/java/org/apache/zookeeper/recipes/leader/LeaderElectionSupport.java

@@ -180,17 +180,25 @@ public class LeaderElectionSupport implements Watcher {
     state = State.OFFER;
     dispatchEvent(EventType.OFFER_START);
 
-    leaderOffer = new LeaderOffer();
-
-    leaderOffer.setHostName(hostName);
-    leaderOffer.setNodePath(zooKeeper.create(rootNodeName + "/" + "n_",
-        hostName.getBytes(), ZooDefs.Ids.OPEN_ACL_UNSAFE,
+    LeaderOffer newLeaderOffer = new LeaderOffer();
+    byte[] hostnameBytes;
+    synchronized (this) {
+        newLeaderOffer.setHostName(hostName);
+        hostnameBytes = hostName.getBytes();
+        newLeaderOffer.setNodePath(zooKeeper.create(rootNodeName + "/" + "n_",
+        hostnameBytes, ZooDefs.Ids.OPEN_ACL_UNSAFE,
         CreateMode.EPHEMERAL_SEQUENTIAL));
-
-    logger.debug("Created leader offer {}", leaderOffer);
-
+        leaderOffer = newLeaderOffer;
+    }
+    
+    logger.debug("Created leader offer {}", newLeaderOffer);
+    
     dispatchEvent(EventType.OFFER_COMPLETE);
   }
+  
+  private synchronized LeaderOffer getLeaderOffer() {
+      return leaderOffer;
+  }
 
   private void determineElectionStatus() throws KeeperException,
       InterruptedException {
@@ -198,9 +206,11 @@ public class LeaderElectionSupport implements Watcher {
     state = State.DETERMINE;
     dispatchEvent(EventType.DETERMINE_START);
 
-    String[] components = leaderOffer.getNodePath().split("/");
+    LeaderOffer currentLeaderOffer = getLeaderOffer();
+    
+    String[] components = currentLeaderOffer.getNodePath().split("/");
 
-    leaderOffer.setId(Integer.valueOf(components[components.length - 1]
+    currentLeaderOffer.setId(Integer.valueOf(components[components.length - 1]
         .substring("n_".length())));
 
     List<LeaderOffer> leaderOffers = toLeaderOffers(zooKeeper.getChildren(
@@ -215,7 +225,7 @@ public class LeaderElectionSupport implements Watcher {
     for (int i = 0; i < leaderOffers.size(); i++) {
       LeaderOffer leaderOffer = leaderOffers.get(i);
 
-      if (leaderOffer.getId().equals(this.leaderOffer.getId())) {
+      if (leaderOffer.getId().equals(currentLeaderOffer.getId())) {
         logger.debug("There are {} leader offers. I am {} in line.",
             leaderOffers.size(), i);
 
@@ -238,7 +248,7 @@ public class LeaderElectionSupport implements Watcher {
     dispatchEvent(EventType.READY_START);
 
     logger.info("{} not elected leader. Watching node:{}",
-        leaderOffer.getNodePath(), neighborLeaderOffer.getNodePath());
+        getLeaderOffer().getNodePath(), neighborLeaderOffer.getNodePath());
 
     /*
      * Make sure to pass an explicit Watcher because we could be sharing this
@@ -270,7 +280,7 @@ public class LeaderElectionSupport implements Watcher {
     state = State.ELECTED;
     dispatchEvent(EventType.ELECTED_START);
 
-    logger.info("Becoming leader with node:{}", leaderOffer.getNodePath());
+    logger.info("Becoming leader with node:{}", getLeaderOffer().getNodePath());
 
     dispatchEvent(EventType.ELECTED_COMPLETE);
   }
@@ -336,7 +346,7 @@ public class LeaderElectionSupport implements Watcher {
   @Override
   public void process(WatchedEvent event) {
     if (event.getType().equals(Watcher.Event.EventType.NodeDeleted)) {
-      if (!event.getPath().equals(leaderOffer.getNodePath())
+      if (!event.getPath().equals(getLeaderOffer().getNodePath())
           && state != State.STOP) {
         logger.debug(
             "Node {} deleted. Need to run through the election process.",
@@ -384,8 +394,8 @@ public class LeaderElectionSupport implements Watcher {
 
   @Override
   public String toString() {
-    return "{ state:" + state + " leaderOffer:" + leaderOffer + " zooKeeper:"
-        + zooKeeper + " hostName:" + hostName + " listeners:" + listeners
+    return "{ state:" + state + " leaderOffer:" + getLeaderOffer() + " zooKeeper:"
+        + zooKeeper + " hostName:" + getHostName() + " listeners:" + listeners
         + " }";
   }
 
@@ -437,11 +447,11 @@ public class LeaderElectionSupport implements Watcher {
    * The hostname of this process. Mostly used as a convenience for logging and
    * to respond to {@link #getLeaderHostName()} requests.
    */
-  public String getHostName() {
+  public synchronized String getHostName() {
     return hostName;
   }
 
-  public void setHostName(String hostName) {
+  public synchronized void setHostName(String hostName) {
     this.hostName = hostName;
   }
 

zookeeper-recipes/zookeeper-recipes-election/src/main/java/org/apache/zookeeper/recipes/leader/LeaderOffer.java

@@ -16,6 +16,7 @@
  */
 package org.apache.zookeeper.recipes.leader;
 
+import java.io.Serializable;
 import java.util.Comparator;
 
 /**
@@ -72,7 +73,8 @@ public class LeaderOffer {
    * Compare two instances of {@link LeaderOffer} using only the {code}id{code}
    * member.
    */
-  public static class IdComparator implements Comparator<LeaderOffer> {
+  public static final class IdComparator
+          implements Comparator<LeaderOffer>, Serializable {
 
     @Override
     public int compare(LeaderOffer o1, LeaderOffer o2) {

zookeeper-recipes/zookeeper-recipes-lock/pom.xml

@@ -47,6 +47,13 @@
       <type>test-jar</type>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>com.github.spotbugs</groupId>
+      <artifactId>spotbugs-annotations</artifactId>
+      <version>3.1.9</version>
+      <scope>provided</scope>
+      <optional>true</optional>
+    </dependency>
   </dependencies>
 
   <build>
@@ -71,4 +78,4 @@
     </plugins>
   </build>
 
-</project>
+</project>

zookeeper-recipes/zookeeper-recipes-lock/src/main/java/org/apache/zookeeper/recipes/lock/WriteLock.java

@@ -17,6 +17,7 @@
  */
 package org.apache.zookeeper.recipes.lock;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import org.apache.zookeeper.KeeperException;
@@ -85,7 +86,7 @@ public class WriteLock extends ProtocolSupport {
      * return the current locklistener
      * @return the locklistener
      */
-    public LockListener getLockListener() {
+    public synchronized LockListener getLockListener() {
         return this.callback;
     }
     
@@ -93,7 +94,7 @@ public class WriteLock extends ProtocolSupport {
      * register a different call back listener
      * @param callback the call back instance
      */
-    public void setLockListener(LockListener callback) {
+    public synchronized void setLockListener(LockListener callback) {
         this.callback = callback;
     }
 
@@ -201,7 +202,9 @@ public class WriteLock extends ProtocolSupport {
          * obtaining the lock
          * @return if the command was successful or not
          */
-        public boolean execute() throws KeeperException, InterruptedException {
+        @SuppressFBWarnings(value = "NP_NULL_PARAM_DEREF_NONVIRTUAL",
+                justification = "findPrefixInChildren will assign a value to this.id")
+        public final boolean execute() throws KeeperException, InterruptedException {
             do {
                 if (id == null) {
                     long sessionId = zookeeper.getSessionId();
@@ -210,45 +213,44 @@ public class WriteLock extends ProtocolSupport {
                     // in the middle of creating the znode
                     findPrefixInChildren(prefix, zookeeper, dir);
                     idName = new ZNodeName(id);
-                }
-                if (id != null) {
-                    List<String> names = zookeeper.getChildren(dir, false);
-                    if (names.isEmpty()) {
-                        LOG.warn("No children in: " + dir + " when we've just " +
-                        "created one! Lets recreate it...");
-                        // lets force the recreation of the id
-                        id = null;
-                    } else {
-                        // lets sort them explicitly (though they do seem to come back in order ususally :)
-                        SortedSet<ZNodeName> sortedNames = new TreeSet<ZNodeName>();
-                        for (String name : names) {
-                            sortedNames.add(new ZNodeName(dir + "/" + name));
+                }                
+                List<String> names = zookeeper.getChildren(dir, false);
+                if (names.isEmpty()) {
+                    LOG.warn("No children in: " + dir + " when we've just " +
+                    "created one! Lets recreate it...");
+                    // lets force the recreation of the id
+                    id = null;
+                } else {
+                    // lets sort them explicitly (though they do seem to come back in order ususally :)
+                    SortedSet<ZNodeName> sortedNames = new TreeSet<ZNodeName>();
+                    for (String name : names) {
+                        sortedNames.add(new ZNodeName(dir + "/" + name));
+                    }
+                    ownerId = sortedNames.first().getName();
+                    SortedSet<ZNodeName> lessThanMe = sortedNames.headSet(idName);
+                    if (!lessThanMe.isEmpty()) {
+                        ZNodeName lastChildName = lessThanMe.last();
+                        lastChildId = lastChildName.getName();
+                        if (LOG.isDebugEnabled()) {
+                            LOG.debug("watching less than me node: " + lastChildId);
                         }
-                        ownerId = sortedNames.first().getName();
-                        SortedSet<ZNodeName> lessThanMe = sortedNames.headSet(idName);
-                        if (!lessThanMe.isEmpty()) {
-                            ZNodeName lastChildName = lessThanMe.last();
-                            lastChildId = lastChildName.getName();
-                            if (LOG.isDebugEnabled()) {
-                                LOG.debug("watching less than me node: " + lastChildId);
-                            }
-                            Stat stat = zookeeper.exists(lastChildId, new LockWatcher());
-                            if (stat != null) {
-                                return Boolean.FALSE;
-                            } else {
-                                LOG.warn("Could not find the" +
-                                		" stats for less than me: " + lastChildName.getName());
-                            }
+                        Stat stat = zookeeper.exists(lastChildId, new LockWatcher());
+                        if (stat != null) {
+                            return Boolean.FALSE;
                         } else {
-                            if (isOwner()) {
-                                if (callback != null) {
-                                    callback.lockAcquired();
-                                }
-                                return Boolean.TRUE;
+                            LOG.warn("Could not find the" +
+                                            " stats for less than me: " + lastChildName.getName());
+                        }
+                    } else {
+                        if (isOwner()) {
+                            LockListener lockListener = getLockListener();
+                            if (lockListener != null) {
+                                lockListener.lockAcquired();
                             }
+                            return Boolean.TRUE;
                         }
                     }
-                }
+                }                
             }
             while (id == null);
             return Boolean.FALSE;
@@ -292,5 +294,6 @@ public class WriteLock extends ProtocolSupport {
     public String getId() {
        return this.id;
     }
+    
 }
 

zookeeper-recipes/zookeeper-recipes-queue/src/main/java/org/apache/zookeeper/recipes/queue/DistributedQueue.java

@@ -86,7 +86,7 @@ public class DistributedQueue {
                     continue;
                 }
                 String suffix = childName.substring(prefix.length());
-                Long childId = new Long(suffix);
+                Long childId = Long.parseLong(suffix);
                 orderedChildren.put(childId,childName);
             }catch(NumberFormatException e){
                 LOG.warn("Found child node with improper format : " + childName + " " + e,e);
@@ -208,7 +208,7 @@ public class DistributedQueue {
         }
     }
 
-    private class LatchChildWatcher implements Watcher {
+    private static class LatchChildWatcher implements Watcher {
 
         CountDownLatch latch;
 

zookeeper-server/pom.xml

@@ -34,6 +34,12 @@
   <description>ZooKeeper server</description>
 
   <dependencies>
+    <dependency>
+      <groupId>com.github.spotbugs</groupId>
+      <artifactId>spotbugs-annotations</artifactId>
+      <scope>provided</scope>
+      <optional>true</optional>
+    </dependency>
     <dependency>
       <groupId>org.hamcrest</groupId>
       <artifactId>hamcrest-all</artifactId>

zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxn.java

@@ -18,6 +18,7 @@
 
 package org.apache.zookeeper;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.io.BufferedReader;
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
@@ -89,6 +90,7 @@ import org.slf4j.LoggerFactory;
  * connected to as needed.
  *
  */
+@SuppressFBWarnings({"EI_EXPOSE_REP", "EI_EXPOSE_REP2"})
 public class ClientCnxn {
     private static final Logger LOG = LoggerFactory.getLogger(ClientCnxn.class);
 
@@ -477,6 +479,7 @@ public class ClientCnxn {
             waitingEvents.add(pair);
         }
 
+       @SuppressFBWarnings("JLM_JSR166_UTILCONCURRENT_MONITORENTER")
        public void queuePacket(Packet packet) {
           if (wasKilled) {
              synchronized (waitingEvents) {
@@ -493,6 +496,7 @@ public class ClientCnxn {
         }
 
         @Override
+        @SuppressFBWarnings("JLM_JSR166_UTILCONCURRENT_MONITORENTER")
         public void run() {
            try {
               isRunning = true;

zookeeper-server/src/main/java/org/apache/zookeeper/ZooDefs.java

@@ -18,6 +18,7 @@
 
 package org.apache.zookeeper;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.util.ArrayList;
 import java.util.Collections;
 
@@ -102,18 +103,21 @@ public class ZooDefs {
         /**
          * This is a completely open ACL .
          */
+        @SuppressFBWarnings(value = "MS_MUTABLE_COLLECTION", justification = "Cannot break API")
         public final ArrayList<ACL> OPEN_ACL_UNSAFE = new ArrayList<ACL>(
                 Collections.singletonList(new ACL(Perms.ALL, ANYONE_ID_UNSAFE)));
 
         /**
          * This ACL gives the creators authentication id's all permissions.
          */
+        @SuppressFBWarnings(value = "MS_MUTABLE_COLLECTION", justification = "Cannot break API")
         public final ArrayList<ACL> CREATOR_ALL_ACL = new ArrayList<ACL>(
                 Collections.singletonList(new ACL(Perms.ALL, AUTH_IDS)));
 
         /**
          * This ACL gives the world the ability to read.
          */
+        @SuppressFBWarnings(value = "MS_MUTABLE_COLLECTION", justification = "Cannot break API")
         public final ArrayList<ACL> READ_ACL_UNSAFE = new ArrayList<ACL>(
                 Collections
                         .singletonList(new ACL(Perms.READ, ANYONE_ID_UNSAFE)));

zookeeper-server/src/main/java/org/apache/zookeeper/server/DataNode.java

@@ -18,6 +18,7 @@
 
 package org.apache.zookeeper.server;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.io.IOException;
 import java.util.HashSet;
 import java.util.Set;
@@ -36,6 +37,7 @@ import org.apache.zookeeper.data.StatPersisted;
  * array of ACLs, a stat object, and a set of its children's paths.
  * 
  */
+@SuppressFBWarnings("EI_EXPOSE_REP2")
 public class DataNode implements Record {
     /** the parent of this datanode */
     DataNode parent;

zookeeper-server/src/main/java/org/apache/zookeeper/server/persistence/FileTxnLog.java

@@ -152,10 +152,10 @@ public class FileTxnLog implements TxnLog {
      * Setter for ServerStats to monitor fsync threshold exceed
      * @param serverStats used to update fsyncThresholdExceedCount
      */
-     @Override
-     public void setServerStats(ServerStats serverStats) {
-         this.serverStats = serverStats;
-     }
+    @Override
+    public synchronized void setServerStats(ServerStats serverStats) {
+        this.serverStats = serverStats;
+    }
 
     /**
      * creates a checksum algorithm to be used

zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/AuthFastLeaderElection.java

@@ -18,6 +18,7 @@
 
 package org.apache.zookeeper.server.quorum;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.io.IOException;
 import java.net.DatagramPacket;
 import java.net.DatagramSocket;
@@ -51,6 +52,8 @@ import org.apache.zookeeper.server.quorum.QuorumPeer.ServerState;
  * @deprecated This class has been deprecated as of release 3.4.0. 
  */
 @Deprecated
+@SuppressFBWarnings(value = "VO_VOLATILE_INCREMENT",
+        justification = "This class is deprecated, no need to fix this warning")
 public class AuthFastLeaderElection implements Election {
     private static final Logger LOG = LoggerFactory.getLogger(AuthFastLeaderElection.class);
 
@@ -276,6 +279,8 @@ public class AuthFastLeaderElection implements Election {
                     case 2:
                         ackstate = QuorumPeer.ServerState.FOLLOWING;
                         break;
+                    default:
+                        throw new IllegalStateException("bad state "+responseBuffer.getInt());
                     }
 
                     Vote current = self.getCurrentVote();
@@ -454,6 +459,8 @@ public class AuthFastLeaderElection implements Election {
                 }
             }
 
+            @SuppressFBWarnings(value = "RV_RETURN_VALUE_IGNORED",
+                    justification = "tryAcquire result not chacked, but it is not an issue")
             private void process(ToSend m) {
                 int attempts = 0;
                 byte zeroes[];
@@ -697,6 +704,9 @@ public class AuthFastLeaderElection implements Election {
                         LOG.warn("Exception while sending ack: ", e);
                     }
                     break;
+                default:
+                    LOG.warn("unknown type " + m.type);
+                    break;
                 }
             }
         }

zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/LearnerSessionTracker.java

@@ -31,7 +31,6 @@ import org.apache.zookeeper.server.ZooKeeperServerListener;
  * to be forwarded to the Leader using a PING.
  */
 public class LearnerSessionTracker implements SessionTracker {
-    SessionExpirer expirer;
 
     HashMap<Long, Integer> touchTable = new HashMap<Long, Integer>();
     long serverId = 1;
@@ -42,7 +41,6 @@ public class LearnerSessionTracker implements SessionTracker {
     public LearnerSessionTracker(SessionExpirer expirer,
             ConcurrentHashMap<Long, Integer> sessionsWithTimeouts, long id,
             ZooKeeperServerListener listener) {
-        this.expirer = expirer;
         this.sessionsWithTimeouts = sessionsWithTimeouts;
         this.serverId = id;
         nextSessionId = SessionTrackerImpl.initializeNextSession(this.serverId);

zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/QuorumPeer.java

@@ -17,6 +17,7 @@
  */
 package org.apache.zookeeper.server.quorum;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.io.BufferedReader;
 import java.io.BufferedWriter;
 import java.io.File;
@@ -1381,6 +1382,8 @@ public class QuorumPeer extends ZooKeeperThread implements QuorumStats.Provider
 	 * @param value the long value to write to the named file
 	 * @throws IOException if the file cannot be written atomically
 	 */
+    @SuppressFBWarnings(value = "OS_OPEN_STREAM",
+            justification = "Stream will be eventually closed in abort()")
     private void writeLongToFile(String name, long value) throws IOException {
         File file = new File(logFactory.getSnapDir(), name);
         AtomicFileOutputStream out = new AtomicFileOutputStream(file);

zookeeper-server/src/main/java/org/apache/zookeeper/server/upgrade/DataNodeV1.java

@@ -18,6 +18,7 @@
 
 package org.apache.zookeeper.server.upgrade;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.io.IOException;
 import java.util.ArrayList;
 import java.util.HashSet;
@@ -71,6 +72,7 @@ public class DataNodeV1 implements Record {
     
     DataNodeV1 parent;
 
+    @SuppressFBWarnings(value="IS2_INCONSISTENT_SYNC")
     byte data[];
 
     public List<ACL> acl;

zookeeper-server/src/main/java/org/apache/zookeeper/server/upgrade/DataTreeV1.java

@@ -414,7 +414,9 @@ public class DataTreeV1 {
                 killSession(header.getClientId());
                 break;
             case OpCode.error:
-                ErrorTxn errTxn = (ErrorTxn) txn;
+                break;
+            default:
+                debug = "Unknown code " + header.getType();
                 break;
             }
         } catch (KeeperException e) {
@@ -539,10 +541,10 @@ public class DataTreeV1 {
         Set<Long> keys = ephemerals.keySet();
         StringBuilder sb = new StringBuilder("Sessions with Ephemerals ("
                 + keys.size() + "):\n");
-        for (long k : keys) {
-            sb.append("0x" + Long.toHexString(k));
+        for (Map.Entry<Long, HashSet<String>> entry : ephemerals.entrySet()) {
+            sb.append("0x" + Long.toHexString(entry.getKey()));
             sb.append(":\n");
-            HashSet<String> tmp = ephemerals.get(k);
+            HashSet<String> tmp = entry.getValue();
             synchronized(tmp) {
                 for (String path : tmp) {
                     sb.append("\t" + path + "\n");