Merge pull request #959 from cshannon/AMQ-9202

AMQ-9202 - Make sure Reentrant locks are acquired outside a try block

activemq-broker/src/main/java/org/apache/activemq/broker/region/Queue.java

@@ -1299,8 +1299,8 @@ public class Queue extends BaseDestination implements Task, UsageListener, Index
     public void purge() throws Exception {
         ConnectionContext c = createConnectionContext();
         List<MessageReference> list = null;
-        try {
         sendLock.lock();
+        try {
             long originalMessageCount = this.destinationStatistics.getMessages().getCount();
             do {
                 doPageIn(true, false, getMaxPageSize());  // signal no expiry processing needed.

activemq-kahadb-store/src/main/java/org/apache/activemq/store/kahadb/MessageDatabase.java

@@ -2070,7 +2070,6 @@ public abstract class MessageDatabase extends ServiceSupport implements BrokerSe
             //flag to know whether the ack forwarding completed without an exception
             boolean forwarded = false;
 
-            try {
             //acquire the checkpoint lock to prevent other threads from
             //running a checkpoint while this is running
             //
@@ -2086,12 +2085,10 @@ public abstract class MessageDatabase extends ServiceSupport implements BrokerSe
             //and only use the executor but this would need to be examined for any unintended
             //consequences
             checkpointLock.readLock().lock();
-
             try {
-
                 // Lock index to capture the ackMessageFileMap data
                 indexLock.writeLock().lock();
-
+                try {
                     // Map keys might not be sorted, find the earliest log file to forward acks
                     // from and move only those, future cycles can chip away at more as needed.
                     // We won't move files that are themselves rewritten on a previous compaction.
@@ -2210,7 +2207,7 @@ public abstract class MessageDatabase extends ServiceSupport implements BrokerSe
 
         // Lock index while we update the ackMessageFileMap.
         indexLock.writeLock().lock();
-
+        try {
             // Update the ack map with the new locations of the acks
             for (Entry<Integer, Set<Integer>> entry : updatedAckLocations.entrySet()) {
                 Set<Integer> referenceFileIds = metadata.ackMessageFileMap.get(entry.getKey());
@@ -2228,8 +2225,9 @@ public abstract class MessageDatabase extends ServiceSupport implements BrokerSe
             // be removed on next GC.
             metadata.ackMessageFileMap.remove(journalToRead);
             metadata.ackMessageFileMapDirtyFlag.lazySet(true);
-
+        } finally {
             indexLock.writeLock().unlock();
+        }
 
         LOG.trace("ACK File Map following updates: {}", metadata.ackMessageFileMap);
     }

activemq-runtime-config/src/main/java/org/apache/activemq/plugin/AbstractRuntimeConfigurationBroker.java

@@ -77,8 +77,8 @@ public class AbstractRuntimeConfigurationBroker extends BrokerFilter {
     public Destination addDestination(ConnectionContext context, ActiveMQDestination destination, boolean createIfTemporary) throws Exception {
         Runnable work = addDestinationWork.poll();
         if (work != null) {
-            try {
             addDestinationBarrier.writeLock().lockInterruptibly();
+            try {
                 do {
                     work.run();
                     work = addDestinationWork.poll();
@@ -88,8 +88,8 @@ public class AbstractRuntimeConfigurationBroker extends BrokerFilter {
                 addDestinationBarrier.writeLock().unlock();
             }
         } else {
-            try {
             addDestinationBarrier.readLock().lockInterruptibly();
+            try {
                 return super.addDestination(context, destination, createIfTemporary);
             } finally {
                 addDestinationBarrier.readLock().unlock();
@@ -102,8 +102,8 @@ public class AbstractRuntimeConfigurationBroker extends BrokerFilter {
     public void addConnection(ConnectionContext context, ConnectionInfo info) throws Exception {
         Runnable work = addConnectionWork.poll();
         if (work != null) {
-            try {
             addConnectionBarrier.writeLock().lockInterruptibly();
+            try {
                 do {
                     work.run();
                     work = addConnectionWork.poll();
@@ -113,8 +113,8 @@ public class AbstractRuntimeConfigurationBroker extends BrokerFilter {
                 addConnectionBarrier.writeLock().unlock();
             }
         } else {
-            try {
             addConnectionBarrier.readLock().lockInterruptibly();
+            try {
                 super.addConnection(context, info);
             } finally {
                 addConnectionBarrier.readLock().unlock();
@@ -131,8 +131,8 @@ public class AbstractRuntimeConfigurationBroker extends BrokerFilter {
     protected void applyDestinationWork() throws Exception {
         Runnable work = addDestinationWork.poll();
         if (work != null) {
-            try {
             addDestinationBarrier.writeLock().lockInterruptibly();
+            try {
                 do {
                     work.run();
                     work = addDestinationWork.poll();