QPID-5038: [Java Broker Tests] Fix leak from message store tests

Ensure that all message store tests close the message store under test on all code paths. This prevents a BDB thread leak manifesting
when running a BDB profile.

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

7 files changed, 80 insertions, 69 deletions

diff --git a/java/bdbstore/src/test/java/org/apache/qpid/server/store/berkeleydb/BDBHAMessageStoreTest.java b/java/bdbstore/src/test/java/org/apache/qpid/server/store/berkeleydb/BDBHAMessageStoreTest.java
index 0bbd399b9f..d7acf27f75 100644
--- a/java/bdbstore/src/test/java/org/apache/qpid/server/store/berkeleydb/BDBHAMessageStoreTest.java
+++ b/java/bdbstore/src/test/java/org/apache/qpid/server/store/berkeleydb/BDBHAMessageStoreTest.java
@@ -77,11 +77,11 @@ public class BDBHAMessageStoreTest extends QpidTestCase
     {
         try
         {
-            FileUtils.delete(new File(_workDir), true);
             if (_virtualHost != null)
             {
                 _virtualHost.close();
             }
+            FileUtils.delete(new File(_workDir), true);
         }
         finally
         {
diff --git a/java/bdbstore/src/test/java/org/apache/qpid/server/store/berkeleydb/BDBMessageStoreConfigurationTest.java b/java/bdbstore/src/test/java/org/apache/qpid/server/store/berkeleydb/BDBMessageStoreConfigurationTest.java
index da1038284e..bfe41773eb 100644
--- a/java/bdbstore/src/test/java/org/apache/qpid/server/store/berkeleydb/BDBMessageStoreConfigurationTest.java
+++ b/java/bdbstore/src/test/java/org/apache/qpid/server/store/berkeleydb/BDBMessageStoreConfigurationTest.java
@@ -25,22 +25,35 @@ import org.apache.qpid.server.store.DurableConfigurationStore;
 
 public class BDBMessageStoreConfigurationTest extends AbstractDurableConfigurationStoreTestCase
 {
-
     private BDBMessageStore _bdbMessageStore;
 
     @Override
-    protected void onReopenStore()
+    protected BDBMessageStore createMessageStore() throws Exception
     {
-        _bdbMessageStore = null;
+        createStoreIfNecessary();
+        return _bdbMessageStore;
     }
 
     @Override
-    protected BDBMessageStore createMessageStore() throws Exception
+    protected void closeMessageStore() throws Exception
+    {
+        closeStoreIfNecessary();
+    }
+
+    @Override
+    protected DurableConfigurationStore createConfigStore() throws Exception
     {
         createStoreIfNecessary();
+
         return _bdbMessageStore;
     }
 
+    @Override
+    protected void closeConfigStore() throws Exception
+    {
+        closeStoreIfNecessary();
+    }
+
     private void createStoreIfNecessary()
     {
         if(_bdbMessageStore == null)
@@ -49,11 +62,12 @@ public class BDBMessageStoreConfigurationTest extends AbstractDurableConfigurati
         }
     }
 
-    @Override
-    protected DurableConfigurationStore createConfigStore() throws Exception
+    private void closeStoreIfNecessary() throws Exception
     {
-        createStoreIfNecessary();
-
-        return _bdbMessageStore;
+        if (_bdbMessageStore != null)
+        {
+            _bdbMessageStore.close();
+            _bdbMessageStore = null;
+        }
     }
 }
diff --git a/java/bdbstore/src/test/java/org/apache/qpid/server/store/berkeleydb/BDBMessageStoreQuotaEventsTest.java b/java/bdbstore/src/test/java/org/apache/qpid/server/store/berkeleydb/BDBMessageStoreQuotaEventsTest.java
index 8ba0d41e03..4684358190 100644
--- a/java/bdbstore/src/test/java/org/apache/qpid/server/store/berkeleydb/BDBMessageStoreQuotaEventsTest.java
+++ b/java/bdbstore/src/test/java/org/apache/qpid/server/store/berkeleydb/BDBMessageStoreQuotaEventsTest.java
@@ -21,9 +21,7 @@
 package org.apache.qpid.server.store.berkeleydb;
 
 import java.util.Collections;
-import java.util.HashMap;
 import java.util.Map;
-import org.apache.commons.configuration.XMLConfiguration;
 import org.apache.log4j.Logger;
 import org.apache.qpid.server.model.VirtualHost;
 import org.apache.qpid.server.store.MessageStore;
diff --git a/java/bdbstore/src/test/java/org/apache/qpid/server/store/berkeleydb/BDBMessageStoreTest.java b/java/bdbstore/src/test/java/org/apache/qpid/server/store/berkeleydb/BDBMessageStoreTest.java
index 85deffd8f2..21342b5715 100644
--- a/java/bdbstore/src/test/java/org/apache/qpid/server/store/berkeleydb/BDBMessageStoreTest.java
+++ b/java/bdbstore/src/test/java/org/apache/qpid/server/store/berkeleydb/BDBMessageStoreTest.java
@@ -42,7 +42,6 @@ import org.apache.qpid.server.message.ServerMessage;
 import org.apache.qpid.server.model.UUIDGenerator;
 import org.apache.qpid.server.protocol.v0_8.MessageMetaDataType_0_8;
 import org.apache.qpid.server.store.MessageStoreTest;
-import org.apache.qpid.server.plugin.MessageMetaDataType;
 import org.apache.qpid.server.store.MessageStore;
 import org.apache.qpid.server.store.StorableMessageMetaData;
 import org.apache.qpid.server.store.StoredMessage;
@@ -57,9 +56,6 @@ import org.apache.qpid.transport.MessageDeliveryPriority;
 import org.apache.qpid.transport.MessageProperties;
 import org.apache.qpid.transport.MessageTransfer;
 
-import static org.mockito.Matchers.eq;
-import static org.mockito.Mockito.mock;
-
 /**
  * Subclass of MessageStoreTest which runs the standard tests from the superclass against
  * the BDB Store as well as additional tests specific to the BDB store-implementation.
@@ -130,12 +126,12 @@ public class BDBMessageStoreTest extends MessageStoreTest
         /*
          * reload the store only (read-only)
          */
-        bdbStore = reloadStore(bdbStore);
+        AbstractBDBMessageStore readOnlyStore = reloadStore(bdbStore);
 
         /*
          * Read back and validate the 0-8 message metadata and content
          */
-        StorableMessageMetaData storeableMMD_0_8 = bdbStore.getMessageMetaData(messageid_0_8);
+        StorableMessageMetaData storeableMMD_0_8 = readOnlyStore.getMessageMetaData(messageid_0_8);
 
         assertEquals("Unexpected message type", MessageMetaDataType_0_8.TYPE, storeableMMD_0_8.getType().ordinal());
         assertTrue("Unexpected instance type", storeableMMD_0_8 instanceof MessageMetaData);
@@ -159,7 +155,7 @@ public class BDBMessageStoreTest extends MessageStoreTest
         assertEquals("Property MessageID has changed", props_0_8.getMessageIdAsString(), returnedProperties_0_8.getMessageIdAsString());
 
         ByteBuffer recoveredContent_0_8 = ByteBuffer.allocate((int) chb_0_8.getBodySize()) ;
-        long recoveredCount_0_8 = bdbStore.getContent(messageid_0_8, 0, recoveredContent_0_8);
+        long recoveredCount_0_8 = readOnlyStore.getContent(messageid_0_8, 0, recoveredContent_0_8);
         assertEquals("Incorrect amount of payload data recovered", chb_0_8.getBodySize(), recoveredCount_0_8);
         String returnedPayloadString_0_8 = new String(recoveredContent_0_8.array());
         assertEquals("Message Payload has changed", bodyText, returnedPayloadString_0_8);
@@ -167,7 +163,7 @@ public class BDBMessageStoreTest extends MessageStoreTest
         /*
          * Read back and validate the 0-10 message metadata and content
          */
-        StorableMessageMetaData storeableMMD_0_10 = bdbStore.getMessageMetaData(messageid_0_10);
+        StorableMessageMetaData storeableMMD_0_10 = readOnlyStore.getMessageMetaData(messageid_0_10);
 
         assertEquals("Unexpected message type", MessageMetaDataType_0_10.TYPE, storeableMMD_0_10.getType().ordinal());
         assertTrue("Unexpected instance type", storeableMMD_0_10 instanceof MessageMetaData_0_10);
@@ -190,11 +186,13 @@ public class BDBMessageStoreTest extends MessageStoreTest
         assertEquals("Message content type has changed", msgProps_0_10.getContentType(), returnedMsgProps.getContentType());
 
         ByteBuffer recoveredContent = ByteBuffer.allocate((int) msgProps_0_10.getContentLength()) ;
-        long recoveredCount = bdbStore.getContent(messageid_0_10, 0, recoveredContent);
+        long recoveredCount = readOnlyStore.getContent(messageid_0_10, 0, recoveredContent);
         assertEquals("Incorrect amount of payload data recovered", msgProps_0_10.getContentLength(), recoveredCount);
 
         String returnedPayloadString_0_10 = new String(recoveredContent.array());
         assertEquals("Message Payload has changed", bodyText, returnedPayloadString_0_10);
+
+        readOnlyStore.close();
     }
 
     private DeliveryProperties createDeliveryProperties_0_10()
diff --git a/java/broker-plugins/derby-store/src/test/java/org/apache/qpid/server/store/derby/DerbyMessageStoreConfigurationTest.java b/java/broker-plugins/derby-store/src/test/java/org/apache/qpid/server/store/derby/DerbyMessageStoreConfigurationTest.java
index ffb6ac479a..4a1a3251e3 100644
--- a/java/broker-plugins/derby-store/src/test/java/org/apache/qpid/server/store/derby/DerbyMessageStoreConfigurationTest.java
+++ b/java/broker-plugins/derby-store/src/test/java/org/apache/qpid/server/store/derby/DerbyMessageStoreConfigurationTest.java
@@ -28,18 +28,17 @@ public class DerbyMessageStoreConfigurationTest extends AbstractDurableConfigura
     private DerbyMessageStore _derbyMessageStore;
 
     @Override
-    protected void onReopenStore()
-    {
-        _derbyMessageStore = null;
-    }
-
-    @Override
     protected DerbyMessageStore createMessageStore() throws Exception
     {
         createStoreIfNecessary();
         return _derbyMessageStore;
     }
 
+    @Override
+    protected void closeMessageStore() throws Exception
+    {
+        closeStoreIfNecessary();
+    }
 
     private void createStoreIfNecessary()
     {
@@ -55,4 +54,19 @@ public class DerbyMessageStoreConfigurationTest extends AbstractDurableConfigura
         createStoreIfNecessary();
         return _derbyMessageStore;
     }
+
+    @Override
+    protected void closeConfigStore() throws Exception
+    {
+        closeStoreIfNecessary();
+    }
+
+    private void closeStoreIfNecessary() throws Exception
+    {
+        if (_derbyMessageStore != null)
+        {
+            _derbyMessageStore.close();
+            _derbyMessageStore = null;
+        }
+    }
 }
diff --git a/java/broker/src/test/java/org/apache/qpid/server/store/AbstractDurableConfigurationStoreTestCase.java b/java/broker/src/test/java/org/apache/qpid/server/store/AbstractDurableConfigurationStoreTestCase.java
index 62c731b2ec..ab1a0f7d0c 100644
--- a/java/broker/src/test/java/org/apache/qpid/server/store/AbstractDurableConfigurationStoreTestCase.java
+++ b/java/broker/src/test/java/org/apache/qpid/server/store/AbstractDurableConfigurationStoreTestCase.java
@@ -116,8 +116,16 @@ public abstract class AbstractDurableConfigurationStoreTestCase extends QpidTest
 
     public void tearDown() throws Exception
     {
-        FileUtils.delete(new File(_storePath), true);
-        super.tearDown();
+        try
+        {
+            closeMessageStore();
+            closeConfigStore();
+            FileUtils.delete(new File(_storePath), true);
+        }
+        finally
+        {
+            super.tearDown();
+        }
     }
 
     public void testCreateExchange() throws Exception
@@ -366,11 +374,8 @@ public abstract class AbstractDurableConfigurationStoreTestCase extends QpidTest
 
     private void reopenStore() throws Exception
     {
-        onReopenStore();
-        if (_messageStore != null)
-        {
-            _messageStore.close();
-        }
+        closeMessageStore();
+        closeConfigStore();
         _messageStore = createMessageStore();
         _configStore = createConfigStore();
 
@@ -379,40 +384,11 @@ public abstract class AbstractDurableConfigurationStoreTestCase extends QpidTest
         _messageStore.activate();
     }
 
-    protected abstract void onReopenStore();
+    protected abstract MessageStore createMessageStore() throws Exception;
+    protected abstract DurableConfigurationStore createConfigStore() throws Exception;
+    protected abstract void closeMessageStore() throws Exception;
+    protected abstract void closeConfigStore() throws Exception;
 
-    abstract protected MessageStore createMessageStore() throws Exception;
-    /*{
-        String storeClass = System.getProperty(MESSAGE_STORE_CLASS_NAME_KEY);
-        if (storeClass == null)
-        {
-            storeClass = DerbyMessageStore.class.getName();
-        }
-        CurrentActor.set(new TestLogActor(new SystemOutMessageLogger()));
-        MessageStore messageStore = (MessageStore) Class.forName(storeClass).newInstance();
-        return messageStore;
-    }
-*/
-    abstract protected DurableConfigurationStore createConfigStore() throws Exception;
-    /*{
-        String storeClass = System.getProperty(CONFIGURATION_STORE_CLASS_NAME_KEY);
-        if (storeClass == null)
-        {
-            storeClass = DerbyMessageStore.class.getName();
-        }
-        Class<DurableConfigurationStore> clazz = (Class<DurableConfigurationStore>) Class.forName(storeClass);
-        DurableConfigurationStore configurationStore ;
-        if(clazz.isInstance(_messageStore))
-        {
-            configurationStore = (DurableConfigurationStore) _messageStore;
-        }
-        else
-        {
-            configurationStore = (DurableConfigurationStore) Class.forName(storeClass).newInstance();
-        }
-        return configurationStore;
-    }
-*/
     public void testRecordXid() throws Exception
     {
         Record enqueueRecord = getTestRecord(1);
diff --git a/java/broker/src/test/java/org/apache/qpid/server/store/MessageStoreQuotaEventsTestBase.java b/java/broker/src/test/java/org/apache/qpid/server/store/MessageStoreQuotaEventsTestBase.java
index 4a246b018b..ea47be83ec 100644
--- a/java/broker/src/test/java/org/apache/qpid/server/store/MessageStoreQuotaEventsTestBase.java
+++ b/java/broker/src/test/java/org/apache/qpid/server/store/MessageStoreQuotaEventsTestBase.java
@@ -70,6 +70,7 @@ public abstract class MessageStoreQuotaEventsTestBase extends QpidTestCase imple
 
         _store = createStore();
         ((DurableConfigurationStore)_store).configureConfigStore("test", null, vhost);
+        _store.configureMessageStore("test", null, null);
 
         _transactionResource = UUID.randomUUID();
         _events = new ArrayList<Event>();
@@ -79,8 +80,18 @@ public abstract class MessageStoreQuotaEventsTestBase extends QpidTestCase imple
     @Override
     public void tearDown() throws Exception
     {
-        super.tearDown();
-        FileUtils.delete(_storeLocation, true);
+        try
+        {
+            super.tearDown();
+        }
+        finally
+        {
+            if (_store != null)
+            {
+                _store.close();
+            }
+            FileUtils.delete(_storeLocation, true);
+        }
     }
 
     public void testOverflow() throws Exception