QPID-2857 : Address issues found by running FindBugs against the Java codebase

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

18 files changed, 72 insertions, 49 deletions

diff --git a/qpid/java/client/example/src/main/java/org/apache/qpid/example/Drain.java b/qpid/java/client/example/src/main/java/org/apache/qpid/example/Drain.java
index 831cea8745..b43031ad23 100644
--- a/qpid/java/client/example/src/main/java/org/apache/qpid/example/Drain.java
+++ b/qpid/java/client/example/src/main/java/org/apache/qpid/example/Drain.java
@@ -100,6 +100,6 @@ public class Drain extends OptionParser
         String u = "Usage: drain [OPTIONS] 'ADDRESS'";
         String d = "Drains messages from the specified address."; 
             
-        Drain drain = new Drain(args,u,d);        
+        new Drain(args,u,d);        
     }
 }
diff --git a/qpid/java/client/example/src/main/java/org/apache/qpid/example/OptionParser.java b/qpid/java/client/example/src/main/java/org/apache/qpid/example/OptionParser.java
index 6b1f514258..f4e17c5c4c 100644
--- a/qpid/java/client/example/src/main/java/org/apache/qpid/example/OptionParser.java
+++ b/qpid/java/client/example/src/main/java/org/apache/qpid/example/OptionParser.java
@@ -69,7 +69,7 @@ public class OptionParser
     
     
     protected Map<String,Object> optMap = new HashMap<String,Object>();
-    protected static List<Option> optDefs = new ArrayList<Option>();
+    protected static final List<Option> optDefs = new ArrayList<Option>();
     
     protected String usage;
     protected String desc;
diff --git a/qpid/java/client/example/src/main/java/org/apache/qpid/example/Spout.java b/qpid/java/client/example/src/main/java/org/apache/qpid/example/Spout.java
index e831df8d28..5da319a658 100644
--- a/qpid/java/client/example/src/main/java/org/apache/qpid/example/Spout.java
+++ b/qpid/java/client/example/src/main/java/org/apache/qpid/example/Spout.java
@@ -143,6 +143,6 @@ public class Spout extends OptionParser
         String u = "Usage: spout [OPTIONS] 'ADDRESS'";
         String d = "Send messages to the specified address."; 
             
-        Spout drain = new Spout(args,u,d);        
+        new Spout(args,u,d);        
     }
 }
diff --git a/qpid/java/client/example/src/main/java/org/apache/qpid/example/shared/InitialContextHelper.java b/qpid/java/client/example/src/main/java/org/apache/qpid/example/shared/InitialContextHelper.java
index 1328816602..16a185133a 100644
--- a/qpid/java/client/example/src/main/java/org/apache/qpid/example/shared/InitialContextHelper.java
+++ b/qpid/java/client/example/src/main/java/org/apache/qpid/example/shared/InitialContextHelper.java
@@ -37,8 +37,8 @@ public class InitialContextHelper
 {
 
     public static final String _defaultPropertiesName = "example.properties";
-    protected static Properties _fileProperties;
-    protected static InitialContext _initialContext;
+    protected Properties _fileProperties;
+    protected InitialContext _initialContext;
     protected static final Logger _log = LoggerFactory.getLogger(InitialContextHelper.class);
 
     public InitialContextHelper(String propertiesName) throws ContextException
diff --git a/qpid/java/client/example/src/main/java/org/apache/qpid/example/subscriber/MonitoredSubscriber.java b/qpid/java/client/example/src/main/java/org/apache/qpid/example/subscriber/MonitoredSubscriber.java
index d43b823a13..e4eb5ac7f5 100644
--- a/qpid/java/client/example/src/main/java/org/apache/qpid/example/subscriber/MonitoredSubscriber.java
+++ b/qpid/java/client/example/src/main/java/org/apache/qpid/example/subscriber/MonitoredSubscriber.java
@@ -33,7 +33,7 @@ public class MonitoredSubscriber extends Subscriber
 
     private static final Logger _logger = LoggerFactory.getLogger(MonitoredSubscriber.class);
 
-    private static MessageConsumer _monitorConsumer;
+    private MessageConsumer _monitorConsumer;
 
     public MonitoredSubscriber()
     {
@@ -107,7 +107,7 @@ public class MonitoredSubscriber extends Subscriber
 
             MonitoredSubscriber._logger.info("Starting monitored subscription ...");
 
-            MonitoredSubscriber._connection.start();
+            _connection.start();
 
             //and now start ordinary consumption too
             subscribe();
diff --git a/qpid/java/client/example/src/main/java/org/apache/qpid/example/subscriber/Subscriber.java b/qpid/java/client/example/src/main/java/org/apache/qpid/example/subscriber/Subscriber.java
index f75558299c..c36668575f 100644
--- a/qpid/java/client/example/src/main/java/org/apache/qpid/example/subscriber/Subscriber.java
+++ b/qpid/java/client/example/src/main/java/org/apache/qpid/example/subscriber/Subscriber.java
@@ -38,13 +38,13 @@ public class Subscriber
 {
     private static final Logger _log = LoggerFactory.getLogger(Subscriber.class);
 
-    protected static Connection _connection;
+    protected Connection _connection;
 
-    protected static MessageConsumer _consumer;
+    protected MessageConsumer _consumer;
 
-    protected static InitialContextHelper _contextHelper;
+    protected InitialContextHelper _contextHelper;
 
-    protected static AMQConnectionFactory _connectionFactory;
+    protected AMQConnectionFactory _connectionFactory;
 
     protected Destination _destination;
 
diff --git a/qpid/java/client/src/main/java/org/apache/qpid/client/AMQBrokerDetails.java b/qpid/java/client/src/main/java/org/apache/qpid/client/AMQBrokerDetails.java
index 6b5673509e..ee52cd50af 100644
--- a/qpid/java/client/src/main/java/org/apache/qpid/client/AMQBrokerDetails.java
+++ b/qpid/java/client/src/main/java/org/apache/qpid/client/AMQBrokerDetails.java
@@ -93,8 +93,8 @@ public class AMQBrokerDetails implements BrokerDetails
 
             if (transport == null)
             {
-                throw URLHelper.parseError(-1, "Unknown transport:'" + transport + "'" +
-                                         " In broker URL:'" + url + "' Format: " + URL_FORMAT_EXAMPLE, "");
+                throw URLHelper.parseError(-1, "Unknown transport in broker URL:'"
+                        + url + "' Format: " + URL_FORMAT_EXAMPLE, "");
             }
 
             setTransport(transport);
@@ -319,6 +319,15 @@ public class AMQBrokerDetails implements BrokerDetails
         //todo do we need to compare all the options as well?
     }
 
+    @Override
+    public int hashCode()
+    {
+        int result = _host != null ? _host.hashCode() : 0;
+        result = 31 * result + _port;
+        result = 31 * result + (_transport != null ? _transport.hashCode() : 0);
+        return result;
+    }
+
     private String printOptionsURL()
     {
         StringBuffer optionsURL = new StringBuffer();
diff --git a/qpid/java/client/src/main/java/org/apache/qpid/client/AMQDestination.java b/qpid/java/client/src/main/java/org/apache/qpid/client/AMQDestination.java
index c6bc1bd622..3a2910732f 100644
--- a/qpid/java/client/src/main/java/org/apache/qpid/client/AMQDestination.java
+++ b/qpid/java/client/src/main/java/org/apache/qpid/client/AMQDestination.java
@@ -123,7 +123,7 @@ public abstract class AMQDestination implements Destination, Referenceable
       }
     }
     
-    protected static DestSyntax defaultDestSyntax;
+    protected final static DestSyntax defaultDestSyntax;
     
     protected DestSyntax _destSyntax;
 
diff --git a/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession.java b/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession.java
index 0f7e0b0812..f54189db6d 100644
--- a/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession.java
+++ b/qpid/java/client/src/main/java/org/apache/qpid/client/AMQSession.java
@@ -150,7 +150,7 @@ public abstract class AMQSession<C extends BasicMessageConsumer, P extends Basic
                 oldVal = _slowAccessConsumers.put(id, consumer);
             }
 
-            return consumer;
+            return oldVal;
 
         }
 
@@ -1959,7 +1959,6 @@ public abstract class AMQSession<C extends BasicMessageConsumer, P extends Basic
 
                         AMQDestination amqd = (AMQDestination) destination;
 
-                        final AMQProtocolHandler protocolHandler = getProtocolHandler();
                         // TODO: Define selectors in AMQP
                         // TODO: construct the rawSelector from the selector string if rawSelector == null
                         final FieldTable ft = FieldTableFactory.newFieldTable();
diff --git a/qpid/java/client/src/main/java/org/apache/qpid/client/handler/ConnectionStartMethodHandler.java b/qpid/java/client/src/main/java/org/apache/qpid/client/handler/ConnectionStartMethodHandler.java
index c9212a54c1..c81ad6422f 100644
--- a/qpid/java/client/src/main/java/org/apache/qpid/client/handler/ConnectionStartMethodHandler.java
+++ b/qpid/java/client/src/main/java/org/apache/qpid/client/handler/ConnectionStartMethodHandler.java
@@ -138,10 +138,9 @@ public class ConnectionStartMethodHandler implements StateAwareMethodListener<Co
 
                 final String locales = new String(body.getLocales(), "utf8");
                 final StringTokenizer tokenizer = new StringTokenizer(locales, " ");
-                String selectedLocale = null;
                 if (tokenizer.hasMoreTokens())
                 {
-                    selectedLocale = tokenizer.nextToken();
+                    tokenizer.nextToken();
                 }
                 else
                 {
diff --git a/qpid/java/client/src/main/java/org/apache/qpid/client/message/AMQMessageDelegate_0_10.java b/qpid/java/client/src/main/java/org/apache/qpid/client/message/AMQMessageDelegate_0_10.java
index 5145c01054..08dd256512 100644
--- a/qpid/java/client/src/main/java/org/apache/qpid/client/message/AMQMessageDelegate_0_10.java
+++ b/qpid/java/client/src/main/java/org/apache/qpid/client/message/AMQMessageDelegate_0_10.java
@@ -579,7 +579,7 @@ public class AMQMessageDelegate_0_10 extends AbstractAMQMessageDelegate
         }
         else
         {
-            return Float.valueOf(null);
+            throw new NullPointerException("No such property: " + propertyName);
         }
 
     }
diff --git a/qpid/java/client/src/main/java/org/apache/qpid/client/message/AMQMessageDelegate_0_8.java b/qpid/java/client/src/main/java/org/apache/qpid/client/message/AMQMessageDelegate_0_8.java
index 1d415b3c5f..cec4268a7b 100644
--- a/qpid/java/client/src/main/java/org/apache/qpid/client/message/AMQMessageDelegate_0_8.java
+++ b/qpid/java/client/src/main/java/org/apache/qpid/client/message/AMQMessageDelegate_0_8.java
@@ -70,7 +70,8 @@ public class AMQMessageDelegate_0_8 extends AbstractAMQMessageDelegate
         _contentHeaderProperties = properties;
         _deliveryTag = deliveryTag;
         _readableProperties = (_contentHeaderProperties != null);
-        _headerAdapter = new JMSHeaderAdapter(((BasicContentHeaderProperties) _contentHeaderProperties).getHeaders());
+        _headerAdapter = new JMSHeaderAdapter(_readableProperties ? ((BasicContentHeaderProperties) _contentHeaderProperties).getHeaders()
+                                                                  : (new BasicContentHeaderProperties()).getHeaders() );
     }
 
     // Used for the creation of new messages
diff --git a/qpid/java/client/src/main/java/org/apache/qpid/client/message/JMSHeaderAdapter.java b/qpid/java/client/src/main/java/org/apache/qpid/client/message/JMSHeaderAdapter.java
index 2e4a6eeb6b..e295d4a2a0 100644
--- a/qpid/java/client/src/main/java/org/apache/qpid/client/message/JMSHeaderAdapter.java
+++ b/qpid/java/client/src/main/java/org/apache/qpid/client/message/JMSHeaderAdapter.java
@@ -236,7 +236,7 @@ public final class JMSHeaderAdapter
             }
             else
             {
-                f = Float.valueOf(null);
+                throw new NullPointerException("No such property: " + string);
             }
 
         }
diff --git a/qpid/java/client/src/main/java/org/apache/qpid/client/protocol/ProtocolBufferMonitorFilter.java b/qpid/java/client/src/main/java/org/apache/qpid/client/protocol/ProtocolBufferMonitorFilter.java
index 93cc5e7ec3..bbd0a7b144 100644
--- a/qpid/java/client/src/main/java/org/apache/qpid/client/protocol/ProtocolBufferMonitorFilter.java
+++ b/qpid/java/client/src/main/java/org/apache/qpid/client/protocol/ProtocolBufferMonitorFilter.java
@@ -36,9 +36,9 @@ public class ProtocolBufferMonitorFilter extends IoFilterAdapter
 {
     private static final Logger _logger = LoggerFactory.getLogger(ProtocolBufferMonitorFilter.class);
 
-    public static long DEFAULT_FREQUENCY = 5000;
+    public static final long DEFAULT_FREQUENCY = 5000;
 
-    public static int DEFAULT_THRESHOLD = 3000;
+    public static final int DEFAULT_THRESHOLD = 3000;
 
     private int _bufferedMessages = 0;
 
diff --git a/qpid/java/client/src/main/java/org/apache/qpid/client/transport/TransportConnection.java b/qpid/java/client/src/main/java/org/apache/qpid/client/transport/TransportConnection.java
index 8e4b59910c..aef3a563af 100644
--- a/qpid/java/client/src/main/java/org/apache/qpid/client/transport/TransportConnection.java
+++ b/qpid/java/client/src/main/java/org/apache/qpid/client/transport/TransportConnection.java
@@ -193,11 +193,14 @@ public class TransportConnection
 
     public static void createVMBroker(int port) throws AMQVMBrokerCreationException
     {
-        if (_acceptor == null)
+        synchronized(TransportConnection.class)
         {
-            _acceptor = new VmPipeAcceptor();
+            if (_acceptor == null)
+            {
+                _acceptor = new VmPipeAcceptor();
 
-            IoServiceConfig config = _acceptor.getDefaultConfig();
+                IoServiceConfig config = _acceptor.getDefaultConfig();
+            }
         }
         synchronized (_inVmPipeAddress)
         {
@@ -313,15 +316,18 @@ public class TransportConnection
     public static void killAllVMBrokers()
     {
         _logger.info("Killing all VM Brokers");
-        if (_acceptor != null)
-        {
-            _acceptor.unbindAll();
-        }
-        synchronized (_inVmPipeAddress)
+        synchronized(TransportConnection.class)
         {
-            _inVmPipeAddress.clear();
+            if (_acceptor != null)
+            {
+                _acceptor.unbindAll();
+            }
+            synchronized (_inVmPipeAddress)
+            {
+                _inVmPipeAddress.clear();
+            }
+            _acceptor = null;
         }
-        _acceptor = null;
         _currentInstance = -1;
         _currentVMPort = -1;
     }
diff --git a/qpid/java/client/src/main/java/org/apache/qpid/client/transport/VmPipeTransportConnection.java b/qpid/java/client/src/main/java/org/apache/qpid/client/transport/VmPipeTransportConnection.java
index 504d475740..87cc2e7a5a 100644
--- a/qpid/java/client/src/main/java/org/apache/qpid/client/transport/VmPipeTransportConnection.java
+++ b/qpid/java/client/src/main/java/org/apache/qpid/client/transport/VmPipeTransportConnection.java
@@ -36,7 +36,7 @@ public class VmPipeTransportConnection implements ITransportConnection
 {
     private static final Logger _logger = LoggerFactory.getLogger(VmPipeTransportConnection.class);
 
-    private static int _port;
+    private int _port;
 
     private MINANetworkDriver _networkDriver;
 
diff --git a/qpid/java/client/src/main/java/org/apache/qpid/client/util/BlockingWaiter.java b/qpid/java/client/src/main/java/org/apache/qpid/client/util/BlockingWaiter.java
index a3d015eadc..208658a5ff 100644
--- a/qpid/java/client/src/main/java/org/apache/qpid/client/util/BlockingWaiter.java
+++ b/qpid/java/client/src/main/java/org/apache/qpid/client/util/BlockingWaiter.java
@@ -242,22 +242,22 @@ public abstract class BlockingWaiter<T>
 
         _lock.lock();
 
-        if (_closed)
+        try
         {
-            return;
-        }
+            if (_closed)
+            {
+                return;
+            }
 
-        if (_error == null)
-        {
-            _error = e;
-        }
-        else
-        {
-            System.err.println("WARNING: new error '" + e == null ? "null" : e.getMessage() + "' arrived while old one not yet processed:" + _error.getMessage());
-        }
+            if (_error == null)
+            {
+                _error = e;
+            }
+            else
+            {
+                System.err.println("WARNING: new error '" + e == null ? "null" : e.getMessage() + "' arrived while old one not yet processed:" + _error.getMessage());
+            }
 
-        try
-        {
             if (_waiting.get())
             {
 
diff --git a/qpid/java/client/src/test/java/org/apache/qpid/test/unit/message/NonQpidMessage.java b/qpid/java/client/src/test/java/org/apache/qpid/test/unit/message/NonQpidMessage.java
index df53c796b2..b1cf23bb9e 100644
--- a/qpid/java/client/src/test/java/org/apache/qpid/test/unit/message/NonQpidMessage.java
+++ b/qpid/java/client/src/test/java/org/apache/qpid/test/unit/message/NonQpidMessage.java
@@ -26,6 +26,7 @@ import java.util.Hashtable;
 import javax.jms.Destination;
 import javax.jms.JMSException;
 import javax.jms.Message;
+import javax.jms.MessageFormatException;
 
 public class NonQpidMessage implements Message
 {
@@ -278,9 +279,17 @@ public class NonQpidMessage implements Message
             {
                 return (Float) o;
             }
+            else if(o instanceof String)
+            {
+                return Float.valueOf((String)o);
+            }
+            else if(o == null)
+            {
+                throw new NullPointerException("No such property: " + string);
+            }
             else
             {
-                return Float.valueOf(null);
+                throw new MessageFormatException("getFloatProperty(\""+string+"\") failed as value is not a float: " + o);
             }
         }
         else
@@ -300,7 +309,7 @@ public class NonQpidMessage implements Message
             }
             else
             {
-                return Double.valueOf(null);
+                return getFloatProperty(string);
             }
         }
         else