QPID-5766: Intitialisation of StatisticsCounter objects can lead to NPE.

Patch Supplied by Andrew MacBean <andymacbean@gmail.com>.

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

9 files changed, 31 insertions, 80 deletions

diff --git a/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/BrokerAdapter.java b/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/BrokerAdapter.java
index 782e4fa4c9..e6d7f44e0c 100644
--- a/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/BrokerAdapter.java
+++ b/java/broker-core/src/main/java/org/apache/qpid/server/model/adapter/BrokerAdapter.java
@@ -82,7 +82,7 @@ public class BrokerAdapter extends AbstractConfiguredObject<BrokerAdapter> imple
     private BrokerOptions _brokerOptions;
 
     private Timer _reportingTimer;
-    private StatisticsCounter _messagesDelivered, _dataDelivered, _messagesReceived, _dataReceived;
+    private final StatisticsCounter _messagesDelivered, _dataDelivered, _messagesReceived, _dataReceived;
 
     @ManagedAttributeField
     private String _defaultVirtualHost;
@@ -119,7 +119,10 @@ public class BrokerAdapter extends AbstractConfiguredObject<BrokerAdapter> imple
             authManager.addUser(BrokerOptions.MANAGEMENT_MODE_USER_NAME, _brokerOptions.getManagementModePassword());
             _managementModeAuthenticationProvider = authManager;
         }
-        initialiseStatistics();
+        _messagesDelivered = new StatisticsCounter("messages-delivered");
+        _dataDelivered = new StatisticsCounter("bytes-delivered");
+        _messagesReceived = new StatisticsCounter("messages-received");
+        _dataReceived = new StatisticsCounter("bytes-received");
     }
 
     public void onValidate()
@@ -243,9 +246,6 @@ public class BrokerAdapter extends AbstractConfiguredObject<BrokerAdapter> imple
             addVirtualHostNode(virtualHostNode);
         }
 
-
-        initialiseStatistics();
-
         initialiseStatisticsReporting();
        // changeChildState(State.ACTIVE, false);
         if (isManagementMode())
@@ -995,14 +995,6 @@ public class BrokerAdapter extends AbstractConfiguredObject<BrokerAdapter> imple
         }
     }
 
-    public void initialiseStatistics()
-    {
-        _messagesDelivered = new StatisticsCounter("messages-delivered");
-        _dataDelivered = new StatisticsCounter("bytes-delivered");
-        _messagesReceived = new StatisticsCounter("messages-received");
-        _dataReceived = new StatisticsCounter("bytes-received");
-    }
-
     private class StatisticsReportingTask extends TimerTask
     {
         private final int DELIVERED = 0;
diff --git a/java/broker-core/src/main/java/org/apache/qpid/server/stats/StatisticsGatherer.java b/java/broker-core/src/main/java/org/apache/qpid/server/stats/StatisticsGatherer.java
index 7fda6862ec..25f5f4e73f 100644
--- a/java/broker-core/src/main/java/org/apache/qpid/server/stats/StatisticsGatherer.java
+++ b/java/broker-core/src/main/java/org/apache/qpid/server/stats/StatisticsGatherer.java
@@ -36,17 +36,6 @@ package org.apache.qpid.server.stats;
 public interface StatisticsGatherer
 {
     /**
-     * Initialise the statistics gathering for this object.
-     * 
-     * This method is responsible for creating any {@link StatisticsCounter}
-     * objects and for determining whether statistics generation should be
-     * enabled, by checking broker and system configuration.
-     * 
-     * @see StatisticsCounter#DISABLE_STATISTICS
-     */
-    void initialiseStatistics();
-    
-    /**
      * This method is responsible for registering the receipt of a message
      * with the counters, and also for passing this notification to any parent
      * {@link StatisticsGatherer}s. If statistics generation is not enabled,
diff --git a/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/AbstractVirtualHost.java b/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/AbstractVirtualHost.java
index 159c526594..cd3d7c889c 100644
--- a/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/AbstractVirtualHost.java
+++ b/java/broker-core/src/main/java/org/apache/qpid/server/virtualhost/AbstractVirtualHost.java
@@ -110,7 +110,7 @@ public abstract class AbstractVirtualHost<X extends AbstractVirtualHost<X>> exte
 
     private volatile VirtualHostState _state = VirtualHostState.INITIALISING;
 
-    private StatisticsCounter _messagesDelivered, _dataDelivered, _messagesReceived, _dataReceived;
+    private final StatisticsCounter _messagesDelivered, _dataDelivered, _messagesReceived, _dataReceived;
 
     private final Map<String, LinkRegistry> _linkRegistry = new HashMap<String, LinkRegistry>();
     private boolean _blocked;
@@ -173,6 +173,10 @@ public abstract class AbstractVirtualHost<X extends AbstractVirtualHost<X>> exte
 
         _defaultDestination = new DefaultDestination(this);
 
+        _messagesDelivered = new StatisticsCounter("messages-delivered-" + getName());
+        _dataDelivered = new StatisticsCounter("bytes-delivered-" + getName());
+        _messagesReceived = new StatisticsCounter("messages-received-" + getName());
+        _dataReceived = new StatisticsCounter("bytes-received-" + getName());
     }
 
     public void onValidate()
@@ -245,8 +249,6 @@ public abstract class AbstractVirtualHost<X extends AbstractVirtualHost<X>> exte
 
         registerSystemNodes();
 
-        initialiseStatistics();
-
         Subject.doAs(SecurityManager.getSubjectWithAddedSystemRights(), new PrivilegedAction<Object>()
         {
             @Override
@@ -790,14 +792,6 @@ public abstract class AbstractVirtualHost<X extends AbstractVirtualHost<X>> exte
         }
     }
 
-    public void initialiseStatistics()
-    {
-        _messagesDelivered = new StatisticsCounter("messages-delivered-" + getName());
-        _dataDelivered = new StatisticsCounter("bytes-delivered-" + getName());
-        _messagesReceived = new StatisticsCounter("messages-received-" + getName());
-        _dataReceived = new StatisticsCounter("bytes-received-" + getName());
-    }
-
     public synchronized LinkRegistry getLinkRegistry(String remoteContainerId)
     {
         LinkRegistry linkRegistry = _linkRegistry.get(remoteContainerId);
diff --git a/java/broker-core/src/test/java/org/apache/qpid/server/consumer/MockConsumer.java b/java/broker-core/src/test/java/org/apache/qpid/server/consumer/MockConsumer.java
index e2f95c6cbe..722535669b 100644
--- a/java/broker-core/src/test/java/org/apache/qpid/server/consumer/MockConsumer.java
+++ b/java/broker-core/src/test/java/org/apache/qpid/server/consumer/MockConsumer.java
@@ -414,10 +414,6 @@ public class MockConsumer implements ConsumerTarget
 
     private static class MockConnectionModel implements AMQConnectionModel
     {
-        @Override
-        public void initialiseStatistics()
-        {
-        }
 
         @Override
         public void registerMessageReceived(long messageSize, long timestamp)
diff --git a/java/broker-core/src/test/java/org/apache/qpid/server/virtualhost/MockVirtualHost.java b/java/broker-core/src/test/java/org/apache/qpid/server/virtualhost/MockVirtualHost.java
index 2877371759..cec34bd890 100644
--- a/java/broker-core/src/test/java/org/apache/qpid/server/virtualhost/MockVirtualHost.java
+++ b/java/broker-core/src/test/java/org/apache/qpid/server/virtualhost/MockVirtualHost.java
@@ -569,11 +569,6 @@ public class MockVirtualHost implements VirtualHostImpl<MockVirtualHost, AMQQueu
         return null;
     }
 
-    public void initialiseStatistics()
-    {
-
-    }
-
     public void registerMessageDelivered(long messageSize)
     {
 
diff --git a/java/broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/ServerConnection.java b/java/broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/ServerConnection.java
index 275ab4416e..ede51ef213 100644
--- a/java/broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/ServerConnection.java
+++ b/java/broker-plugins/amqp-0-10-protocol/src/main/java/org/apache/qpid/server/protocol/v0_10/ServerConnection.java
@@ -71,7 +71,7 @@ public class ServerConnection extends Connection implements AMQConnectionModel<S
 
     private final Subject _authorizedSubject = new Subject();
     private Principal _authorizedPrincipal = null;
-    private StatisticsCounter _messagesDelivered, _dataDelivered, _messagesReceived, _dataReceived;
+    private final StatisticsCounter _messagesDelivered, _dataDelivered, _messagesReceived, _dataReceived;
     private final long _connectionId;
     private final Object _reference = new Object();
     private VirtualHostImpl _virtualHost;
@@ -93,6 +93,11 @@ public class ServerConnection extends Connection implements AMQConnectionModel<S
         _connectionId = connectionId;
         _authorizedSubject.getPrincipals().add(new ConnectionPrincipal(this));
         _broker = broker;
+
+        _messagesDelivered = new StatisticsCounter("messages-delivered-" + getConnectionId());
+        _dataDelivered = new StatisticsCounter("data-delivered-" + getConnectionId());
+        _messagesReceived = new StatisticsCounter("messages-received-" + getConnectionId());
+        _dataReceived = new StatisticsCounter("data-received-" + getConnectionId());
     }
 
     public Object getReference()
@@ -175,8 +180,6 @@ public class ServerConnection extends Connection implements AMQConnectionModel<S
     public void setVirtualHost(VirtualHostImpl virtualHost)
     {
         _virtualHost = virtualHost;
-
-        initialiseStatistics();
     }
 
     @Override
@@ -455,14 +458,6 @@ public class ServerConnection extends Connection implements AMQConnectionModel<S
         _dataReceived.reset();
     }
 
-    public void initialiseStatistics()
-    {
-        _messagesDelivered = new StatisticsCounter("messages-delivered-" + getConnectionId());
-        _dataDelivered = new StatisticsCounter("data-delivered-" + getConnectionId());
-        _messagesReceived = new StatisticsCounter("messages-received-" + getConnectionId());
-        _dataReceived = new StatisticsCounter("data-received-" + getConnectionId());
-    }
-
     /**
      * @return authorizedSubject
      */
diff --git a/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/AMQProtocolEngine.java b/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/AMQProtocolEngine.java
index f6ad008441..b28e9bc23c 100644
--- a/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/AMQProtocolEngine.java
+++ b/java/broker-plugins/amqp-0-8-protocol/src/main/java/org/apache/qpid/server/protocol/v0_8/AMQProtocolEngine.java
@@ -154,7 +154,7 @@ public class AMQProtocolEngine implements ServerProtocolEngine, AMQProtocolSessi
     private long _maxFrameSize;
     private final AtomicBoolean _closing = new AtomicBoolean(false);
 
-    private StatisticsCounter _messagesDelivered, _dataDelivered, _messagesReceived, _dataReceived;
+    private final StatisticsCounter _messagesDelivered, _dataDelivered, _messagesReceived, _dataReceived;
 
     private NetworkConnection _network;
     private Sender<ByteBuffer> _sender;
@@ -201,12 +201,14 @@ public class AMQProtocolEngine implements ServerProtocolEngine, AMQProtocolSessi
 
                 _closeWhenNoRoute = _broker.getConnection_closeWhenNoRoute();
 
-                initialiseStatistics();
-
                 return null;
             }
         });
 
+        _messagesDelivered = new StatisticsCounter("messages-delivered-" + getSessionID());
+        _dataDelivered = new StatisticsCounter("data-delivered-" + getSessionID());
+        _messagesReceived = new StatisticsCounter("messages-received-" + getSessionID());
+        _dataReceived = new StatisticsCounter("data-received-" + getSessionID());
     }
 
     private <T> T runAsSubject(PrivilegedAction<T> action)
@@ -1500,14 +1502,6 @@ public class AMQProtocolEngine implements ServerProtocolEngine, AMQProtocolSessi
         _dataReceived.reset();
     }
 
-    public void initialiseStatistics()
-    {
-        _messagesDelivered = new StatisticsCounter("messages-delivered-" + getSessionID());
-        _dataDelivered = new StatisticsCounter("data-delivered-" + getSessionID());
-        _messagesReceived = new StatisticsCounter("messages-received-" + getSessionID());
-        _dataReceived = new StatisticsCounter("data-received-" + getSessionID());
-    }
-
     public boolean isSessionNameUnique(byte[] name)
     {
         // 0-8/0-9/0-9-1 sessions don't have names
diff --git a/java/broker-plugins/amqp-1-0-protocol/src/main/java/org/apache/qpid/server/protocol/v1_0/Connection_1_0.java b/java/broker-plugins/amqp-1-0-protocol/src/main/java/org/apache/qpid/server/protocol/v1_0/Connection_1_0.java
index 5b8d3a488e..6d780461f4 100644
--- a/java/broker-plugins/amqp-1-0-protocol/src/main/java/org/apache/qpid/server/protocol/v1_0/Connection_1_0.java
+++ b/java/broker-plugins/amqp-1-0-protocol/src/main/java/org/apache/qpid/server/protocol/v1_0/Connection_1_0.java
@@ -76,10 +76,7 @@ public class Connection_1_0 implements ConnectionEventListener, AMQConnectionMod
     private final CopyOnWriteArrayList<SessionModelListener> _sessionListeners =
             new CopyOnWriteArrayList<SessionModelListener>();
 
-    private StatisticsCounter _messageDeliveryStatistics = new StatisticsCounter();
-    private StatisticsCounter _messageReceiptStatistics = new StatisticsCounter();
-    private StatisticsCounter _dataDeliveryStatistics = new StatisticsCounter();
-    private StatisticsCounter _dataReceiptStatistics = new StatisticsCounter();
+    private final StatisticsCounter _messageDeliveryStatistics, _messageReceiptStatistics, _dataDeliveryStatistics, _dataReceiptStatistics;
 
     private final LogSubject _logSubject = new LogSubject()
     {
@@ -118,7 +115,10 @@ public class Connection_1_0 implements ConnectionEventListener, AMQConnectionMod
         _connectionId = connectionId;
         _subject.getPrincipals().add(new ConnectionPrincipal(this));
         _subjectCreator = subjectCreator;
-
+        _messageDeliveryStatistics = new StatisticsCounter("messages-delivered-" + getConnectionId());
+        _dataDeliveryStatistics = new StatisticsCounter("data-delivered-" + getConnectionId());
+        _messageReceiptStatistics = new StatisticsCounter("messages-received-" + getConnectionId());
+        _dataReceiptStatistics = new StatisticsCounter("data-received-" + getConnectionId());
     }
 
     public Object getReference()
@@ -368,15 +368,6 @@ public class Connection_1_0 implements ConnectionEventListener, AMQConnectionMod
     }
 
     @Override
-    public void initialiseStatistics()
-    {
-        _messageDeliveryStatistics = new StatisticsCounter("messages-delivered-" + getConnectionId());
-        _dataDeliveryStatistics = new StatisticsCounter("data-delivered-" + getConnectionId());
-        _messageReceiptStatistics = new StatisticsCounter("messages-received-" + getConnectionId());
-        _dataReceiptStatistics = new StatisticsCounter("data-received-" + getConnectionId());
-    }
-
-    @Override
     public void registerMessageReceived(long messageSize, long timestamp)
     {
         _messageReceiptStatistics.registerEvent(1L, timestamp);
diff --git a/java/systests/src/main/java/org/apache/qpid/client/ssl/SSLTest.java b/java/systests/src/main/java/org/apache/qpid/client/ssl/SSLTest.java
index a7fd7d362d..8225fce3a3 100644
--- a/java/systests/src/main/java/org/apache/qpid/client/ssl/SSLTest.java
+++ b/java/systests/src/main/java/org/apache/qpid/client/ssl/SSLTest.java
@@ -43,9 +43,13 @@ import org.apache.qpid.server.model.Port;
 import org.apache.qpid.server.model.Transport;
 import org.apache.qpid.test.utils.QpidBrokerTestCase;
 import org.apache.qpid.test.utils.TestBrokerConfiguration;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 public class SSLTest extends QpidBrokerTestCase
 {
+    private static final Logger LOGGER = LoggerFactory.getLogger(SSLTest.class);
+
     private static final String CERT_ALIAS_APP1 = "app1";
     private static final String CERT_ALIAS_APP2 = "app2";
 
@@ -209,6 +213,7 @@ public class SSLTest extends QpidBrokerTestCase
 
     private void verifyExceptionCausesContains(Exception e, String expectedString)
     {
+        LOGGER.debug("verifying that the following exception contains " + expectedString, e);
         ByteArrayOutputStream bout = new ByteArrayOutputStream();
         e.printStackTrace(new PrintStream(bout));
         String strace = bout.toString();