zookeeper-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From an...@apache.org
Subject [zookeeper] branch master updated: ZOOKEEPER-3223: Configure Spotbugs
Date Mon, 07 Jan 2019 13:24:22 GMT
This is an automated email from the ASF dual-hosted git repository.

andor pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/zookeeper.git


The following commit(s) were added to refs/heads/master by this push:
     new b752ef6  ZOOKEEPER-3223: Configure Spotbugs
b752ef6 is described below

commit b752ef66876a141035a42f30aad69e3166cad746
Author: Enrico Olivelli <eolivelli@apache.org>
AuthorDate: Mon Jan 7 14:24:07 2019 +0100

    ZOOKEEPER-3223: Configure Spotbugs
    
    - add spotbugs configuration (default)
    - make build pass spotbugs
    
    Author: Enrico Olivelli <eolivelli@apache.org>
    
    Reviewers: andor@apache.org
    
    Closes #742 from eolivelli/fix/ZOOKEEPER-3223-spotbugs and squashes the following commits:
    
    a43cecf41 [Enrico Olivelli] Fix false positive
    c00d296ad [Enrico Olivelli] Add Suppression for false positive
    35c8a4dde [Enrico Olivelli] fix tests
    1ae629bcd [Enrico Olivelli] revert file
    c0bb9d903 [Enrico Olivelli] Add spotbugs annotations to ant based build
    dabe4fafc [Enrico Olivelli] [ZOOKEEPER-3223] Configure Spotbugs - add spotbugs configuration
- make build pass spotbugs
---
 build.xml                                          |  1 +
 excludeFindBugsFilter.xml                          | 14 ++++++++++++++
 ivy.xml                                            |  1 +
 pom.xml                                            | 22 +++++++++++++++++++++-
 zookeeper-jute/pom.xml                             |  8 ++++++++
 zookeeper-server/pom.xml                           |  6 ++++++
 .../main/java/org/apache/zookeeper/ClientCnxn.java |  4 ++++
 .../main/java/org/apache/zookeeper/ZooDefs.java    |  4 ++++
 .../java/org/apache/zookeeper/server/DataNode.java |  2 ++
 .../org/apache/zookeeper/server/EphemeralType.java |  3 +++
 .../zookeeper/server/persistence/FileTxnLog.java   |  2 +-
 .../server/quorum/AuthFastLeaderElection.java      |  3 +++
 .../zookeeper/server/quorum/CommitProcessor.java   |  3 +++
 .../zookeeper/server/quorum/ObserverMaster.java    |  5 +++++
 .../apache/zookeeper/server/util/BitHashSet.java   |  7 +++++--
 .../org/apache/zookeeper/server/util/BitMap.java   |  3 +++
 16 files changed, 84 insertions(+), 4 deletions(-)

diff --git a/build.xml b/build.xml
index 50bc94f..265a284 100644
--- a/build.xml
+++ b/build.xml
@@ -28,6 +28,7 @@ xmlns:cs="antlib:com.puppycrawl.tools.checkstyle.ant">
     <!-- ====================================================== -->
     <property name="slf4j.version" value="1.7.25"/>
     <property name="commons-cli.version" value="1.2"/>
+    <property name="spotbugsannotations.version" value="3.1.8"/>
 
     <property name="wagon-http.version" value="2.4"/>
     <property name="maven-ant-tasks.version" value="2.1.3"/>
diff --git a/excludeFindBugsFilter.xml b/excludeFindBugsFilter.xml
new file mode 100644
index 0000000..c836911
--- /dev/null
+++ b/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>
+
diff --git a/ivy.xml b/ivy.xml
index c7f79b6..a953317 100644
--- a/ivy.xml
+++ b/ivy.xml
@@ -46,6 +46,7 @@
     <dependency org="org.slf4j" name="slf4j-api" rev="${slf4j.version}"/>
     <dependency org="org.slf4j" name="slf4j-log4j12" rev="${slf4j.version}" transitive="false"/>
     <dependency org="commons-cli" name="commons-cli" rev="${commons-cli.version}" />
+    <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"/>
diff --git a/pom.xml b/pom.xml
index 58c9e50..0dc7174 100755
--- a/pom.xml
+++ b/pom.xml
@@ -276,6 +276,7 @@
     <kerby.version>1.1.0</kerby.version>
     <bouncycastle.version>1.56</bouncycastle.version>
     <commons-collections.version>3.2.2</commons-collections.version>
+    <spotbugsannotations.version>3.1.8</spotbugsannotations.version>
   </properties>
 
   <dependencyManagement>
@@ -408,6 +409,13 @@
         <artifactId>jline</artifactId>
         <version>${jline.version}</version>
       </dependency>
+      <dependency>
+         <groupId>com.github.spotbugs</groupId>
+         <artifactId>spotbugs-annotations</artifactId>
+         <version>${spotbugsannotations.version}</version>
+         <scope>provided</scope>
+         <optional>true</optional>
+        </dependency>
     </dependencies>
   </dependencyManagement>
 
@@ -459,6 +467,14 @@
           <artifactId>clover-maven-plugin</artifactId>
           <version>4.3.1</version>
         </plugin>
+        <plugin>
+          <groupId>com.github.spotbugs</groupId>
+          <artifactId>spotbugs-maven-plugin</artifactId>
+          <version>3.1.8</version>
+          <configuration>
+            <excludeFilterFile>excludeFindBugsFilter.xml</excludeFilterFile>
+          </configuration>
+        </plugin>
       </plugins>
     </pluginManagement>
 
@@ -486,7 +502,11 @@
           </execution>
         </executions>
       </plugin>
-    </plugins>
+      <plugin>
+        <groupId>com.github.spotbugs</groupId>
+        <artifactId>spotbugs-maven-plugin</artifactId>
+      </plugin>
+      </plugins>
   </build>
 
   <reporting>
diff --git a/zookeeper-jute/pom.xml b/zookeeper-jute/pom.xml
index bc133b2..9bb696e 100755
--- a/zookeeper-jute/pom.xml
+++ b/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>
 
diff --git a/zookeeper-server/pom.xml b/zookeeper-server/pom.xml
index b4ee890..9062469 100755
--- a/zookeeper-server/pom.xml
+++ b/zookeeper-server/pom.xml
@@ -35,6 +35,12 @@
 
   <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>
       <scope>test</scope>
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxn.java b/zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxn.java
index ef53edf..db2b486 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/ClientCnxn.java
+++ b/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;
@@ -97,6 +98,7 @@ import org.slf4j.MDC;
  * connected to as needed.
  *
  */
+@SuppressFBWarnings({"EI_EXPOSE_REP", "EI_EXPOSE_REP2"})
 public class ClientCnxn {
     private static final Logger LOG = LoggerFactory.getLogger(ClientCnxn.class);
 
@@ -479,6 +481,7 @@ public class ClientCnxn {
             waitingEvents.add(new LocalCallback(cb, rc, path, ctx));
         }
 
+       @SuppressFBWarnings("JLM_JSR166_UTILCONCURRENT_MONITORENTER")
        public void queuePacket(Packet packet) {
           if (wasKilled) {
              synchronized (waitingEvents) {
@@ -495,6 +498,7 @@ public class ClientCnxn {
         }
 
         @Override
+        @SuppressFBWarnings("JLM_JSR166_UTILCONCURRENT_MONITORENTER")
         public void run() {
            try {
               isRunning = true;
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/ZooDefs.java b/zookeeper-server/src/main/java/org/apache/zookeeper/ZooDefs.java
index f685e32..97aa28a 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/ZooDefs.java
+++ b/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;
 
@@ -118,18 +119,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)));
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/DataNode.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/DataNode.java
index 5922d16..550b151 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/DataNode.java
+++ b/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 data for this datanode */
     byte data[];
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/EphemeralType.java
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/EphemeralType.java
index f5d58ae..d4c6c80 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/EphemeralType.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/EphemeralType.java
@@ -18,6 +18,7 @@
 
 package org.apache.zookeeper.server;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import org.apache.zookeeper.CreateMode;
 
 import java.util.Collections;
@@ -210,6 +211,8 @@ public enum EphemeralType {
      * @param ttl  ttl
      * @throws IllegalArgumentException if the ttl is not valid for the mode
      */
+    @SuppressFBWarnings(value = "RV_RETURN_VALUE_IGNORED_NO_SIDE_EFFECT",
+            justification = "toEphemeralOwner may throw IllegalArgumentException")
     public static void validateTTL(CreateMode mode, long ttl) {
         if (mode.isTTL()) {
             TTL.toEphemeralOwner(ttl);
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/persistence/FileTxnLog.java
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/persistence/FileTxnLog.java
index d95dac8..c739152 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/persistence/FileTxnLog.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/persistence/FileTxnLog.java
@@ -180,7 +180,7 @@ public class FileTxnLog implements TxnLog {
      * @param serverStats used to update fsyncThresholdExceedCount
      */
     @Override
-    public void setServerStats(ServerStats serverStats) {
+    public synchronized void setServerStats(ServerStats serverStats) {
         this.serverStats = serverStats;
     }
 
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/AuthFastLeaderElection.java
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/AuthFastLeaderElection.java
index 9347152..933cbfd 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/AuthFastLeaderElection.java
+++ b/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;
@@ -460,6 +461,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[];
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/CommitProcessor.java
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/CommitProcessor.java
index 1eb7fac..9982f15 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/CommitProcessor.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/CommitProcessor.java
@@ -18,6 +18,7 @@
 
 package org.apache.zookeeper.server.quorum;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.io.IOException;
 import java.util.HashMap;
 import java.util.LinkedList;
@@ -395,6 +396,7 @@ public class CommitProcessor extends ZooKeeperCriticalThread implements
         }
     }
 
+    @SuppressFBWarnings("NN_NAKED_NOTIFY")
     synchronized private void wakeup() {
         notifyAll();
     }
@@ -416,6 +418,7 @@ public class CommitProcessor extends ZooKeeperCriticalThread implements
         wakeup();
     }
 
+    @Override
     public void processRequest(Request request) {
         if (stopped) {
             return;
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/ObserverMaster.java
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/ObserverMaster.java
index 07de57b..7308f65 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/ObserverMaster.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/ObserverMaster.java
@@ -461,9 +461,14 @@ public class ObserverMaster implements LearnerMaster, Runnable {
     }
 
     public void run() {
+        ServerSocket ss;
+        synchronized(this) {
+             ss = this.ss;
+        }
         while (listenerRunning) {
             try {
                 Socket s = ss.accept();
+
                 // start with the initLimit, once the ack is processed
                 // in LearnerHandler switch to the syncLimit
                 s.setSoTimeout(self.tickTime * self.initLimit);
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/util/BitHashSet.java
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/util/BitHashSet.java
index b60f1d4..a8de793 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/util/BitHashSet.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/util/BitHashSet.java
@@ -120,7 +120,10 @@ public class BitHashSet implements Iterable<Integer> {
      */
     @Override
     public Iterator<Integer> iterator() {
-        if (cache.size() == elementCount) {
+        // sample current size at the beginning
+        int currentSize = size();
+
+        if (cache.size() == currentSize) {
             return cache.iterator();
         }
 
@@ -130,7 +133,7 @@ public class BitHashSet implements Iterable<Integer> {
 
             @Override
             public boolean hasNext() {
-                return returnedCount < elementCount;
+                return returnedCount < currentSize;
             }
 
             @Override
diff --git a/zookeeper-server/src/main/java/org/apache/zookeeper/server/util/BitMap.java b/zookeeper-server/src/main/java/org/apache/zookeeper/server/util/BitMap.java
index 1a0fb3b..691c5a7 100644
--- a/zookeeper-server/src/main/java/org/apache/zookeeper/server/util/BitMap.java
+++ b/zookeeper-server/src/main/java/org/apache/zookeeper/server/util/BitMap.java
@@ -18,6 +18,7 @@
 
 package org.apache.zookeeper.server.util;
 
+import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
 import java.util.Map;
 import java.util.HashMap;
 import java.util.BitSet;
@@ -37,6 +38,8 @@ public class BitMap<T> {
 
     private final ReentrantReadWriteLock rwLock = new ReentrantReadWriteLock();
 
+    @SuppressFBWarnings(value = "DLS_DEAD_LOCAL_STORE",
+            justification = "SpotBugs false positive")
     public Integer add(T value) {
         /*
          * Optimized for code which will add the same value again and again,


Mime
View raw message