You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zookeeper.apache.org by GitBox <gi...@apache.org> on 2019/01/07 13:24:44 UTC
[GitHub] asfgit closed pull request #742: ZOOKEEPER-3223: Configure Spotbugs
asfgit closed pull request #742: ZOOKEEPER-3223: Configure Spotbugs
URL: https://github.com/apache/zookeeper/pull/742
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/build.xml b/build.xml
index f8a0546740..dca2e0bf49 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 0000000000..c836911dbe
--- /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 c7f79b6634..a95331780d 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 58c9e50d36..0dc71743c5 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 bc133b29b7..9bb696e9a7 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 b4ee890d2a..9062469352 100755
--- a/zookeeper-server/pom.xml
+++ b/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>
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 ef53edf0a8..db2b4866af 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 @@
* 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 void queueCallback(AsyncCallback cb, int rc, String path,
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 void queueEventOfDeath() {
}
@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 f685e32e2a..97aa28a7c7 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 @@
/**
* 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 5922d16584..550b151529 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 @@
* 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 f5d58ae8be..d4c6c8086e 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 static void validateServerId(long serverId) {
* @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 d95dac8464..c739152799 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 static void setPreallocSize(long size) {
* @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 93471526d2..933cbfd486 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 void run() {
}
}
+ @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 1eb7faca56..9982f15061 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 void doWork() throws RequestProcessorException {
}
}
+ @SuppressFBWarnings("NN_NAKED_NOTIFY")
synchronized private void wakeup() {
notifyAll();
}
@@ -416,6 +418,7 @@ public void commit(Request request) {
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 07de57be86..7308f6597a 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 @@ synchronized public void start() throws IOException {
}
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 b60f1d4754..a8de793c25 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 synchronized int size() {
*/
@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 synchronized int size() {
@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 1a0fb3bac5..691c5a7293 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 @@
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,
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services