This is an automated email from the ASF dual-hosted git repository.
clebertsuconic pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/activemq-artemis.git
The following commit(s) were added to refs/heads/master by this push:
new f155838 ARTEMIS-2313 Accumulation in HierarchicalObjectRepository
cache
new aa0bf60 This closes #2628
f155838 is described below
commit f155838626827b3bf5ed26fc41fe4e90601879fe
Author: Justin Bertram <[email protected]>
AuthorDate: Thu Apr 18 13:34:01 2019 -0500
ARTEMIS-2313 Accumulation in HierarchicalObjectRepository cache
---
.../core/postoffice/impl/PostOfficeImpl.java | 77 +++++++++---------
.../artemis/core/server/ActiveMQServer.java | 2 +
.../core/server/impl/ActiveMQServerImpl.java | 90 +++++++++++-----------
.../jms/client/TemporaryDestinationTest.java | 30 ++++++++
4 files changed, 119 insertions(+), 80 deletions(-)
diff --git
a/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java
b/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java
index b88c6a0..ad839c1 100644
---
a/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java
+++
b/artemis-server/src/main/java/org/apache/activemq/artemis/core/postoffice/impl/PostOfficeImpl.java
@@ -725,61 +725,64 @@ public class PostOfficeImpl implements PostOffice,
NotificationListener, Binding
server.callBrokerBindingPlugins(plugin ->
plugin.beforeRemoveBinding(uniqueName, tx, deleteData));
}
- addressSettingsRepository.clearCache();
+ try {
- Binding binding = addressManager.removeBinding(uniqueName, tx);
+ Binding binding = addressManager.removeBinding(uniqueName, tx);
- if (binding == null) {
- throw new ActiveMQNonExistentQueueException();
- }
+ if (binding == null) {
+ throw new ActiveMQNonExistentQueueException();
+ }
- if (deleteData &&
addressManager.getBindingsForRoutingAddress(binding.getAddress()) == null) {
- pagingManager.deletePageStore(binding.getAddress());
+ if (deleteData &&
addressManager.getBindingsForRoutingAddress(binding.getAddress()) == null) {
+ pagingManager.deletePageStore(binding.getAddress());
- deleteDuplicateCache(binding.getAddress());
- }
+ deleteDuplicateCache(binding.getAddress());
+ }
- if (binding.getType() == BindingType.LOCAL_QUEUE) {
- Queue queue = (Queue) binding.getBindable();
- managementService.unregisterQueue(uniqueName, binding.getAddress(),
queue.getRoutingType());
- } else if (binding.getType() == BindingType.DIVERT) {
- managementService.unregisterDivert(uniqueName, binding.getAddress());
- }
+ if (binding.getType() == BindingType.LOCAL_QUEUE) {
+ Queue queue = (Queue) binding.getBindable();
+ managementService.unregisterQueue(uniqueName,
binding.getAddress(), queue.getRoutingType());
+ } else if (binding.getType() == BindingType.DIVERT) {
+ managementService.unregisterDivert(uniqueName,
binding.getAddress());
+ }
- AddressInfo addressInfo = getAddressInfo(binding.getAddress());
- if (addressInfo != null) {
- addressInfo.setBindingRemovedTimestamp(System.currentTimeMillis());
- }
+ AddressInfo addressInfo = getAddressInfo(binding.getAddress());
+ if (addressInfo != null) {
+ addressInfo.setBindingRemovedTimestamp(System.currentTimeMillis());
+ }
- if (binding.getType() != BindingType.DIVERT) {
- TypedProperties props = new TypedProperties();
+ if (binding.getType() != BindingType.DIVERT) {
+ TypedProperties props = new TypedProperties();
- props.putSimpleStringProperty(ManagementHelper.HDR_ADDRESS,
binding.getAddress());
+ props.putSimpleStringProperty(ManagementHelper.HDR_ADDRESS,
binding.getAddress());
- props.putSimpleStringProperty(ManagementHelper.HDR_CLUSTER_NAME,
binding.getClusterName());
+ props.putSimpleStringProperty(ManagementHelper.HDR_CLUSTER_NAME,
binding.getClusterName());
- props.putSimpleStringProperty(ManagementHelper.HDR_ROUTING_NAME,
binding.getRoutingName());
+ props.putSimpleStringProperty(ManagementHelper.HDR_ROUTING_NAME,
binding.getRoutingName());
- props.putIntProperty(ManagementHelper.HDR_DISTANCE,
binding.getDistance());
+ props.putIntProperty(ManagementHelper.HDR_DISTANCE,
binding.getDistance());
- props.putLongProperty(ManagementHelper.HDR_BINDING_ID,
binding.getID());
+ props.putLongProperty(ManagementHelper.HDR_BINDING_ID,
binding.getID());
- if (binding.getFilter() == null) {
- props.putSimpleStringProperty(ManagementHelper.HDR_FILTERSTRING,
null);
- } else {
- props.putSimpleStringProperty(ManagementHelper.HDR_FILTERSTRING,
binding.getFilter().getFilterString());
+ if (binding.getFilter() == null) {
+
props.putSimpleStringProperty(ManagementHelper.HDR_FILTERSTRING, null);
+ } else {
+
props.putSimpleStringProperty(ManagementHelper.HDR_FILTERSTRING,
binding.getFilter().getFilterString());
+ }
+
+ managementService.sendNotification(new Notification(null,
CoreNotificationType.BINDING_REMOVED, props));
}
- managementService.sendNotification(new Notification(null,
CoreNotificationType.BINDING_REMOVED, props));
- }
+ binding.close();
- binding.close();
+ if (server.hasBrokerBindingPlugins()) {
+ server.callBrokerBindingPlugins(plugin ->
plugin.afterRemoveBinding(binding, tx, deleteData));
+ }
- if (server.hasBrokerBindingPlugins()) {
- server.callBrokerBindingPlugins(plugin ->
plugin.afterRemoveBinding(binding, tx, deleteData) );
+ return binding;
+ } finally {
+ server.clearAddressCache();
}
-
- return binding;
}
private void deleteDuplicateCache(SimpleString address) throws Exception {
diff --git
a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQServer.java
b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQServer.java
index 5d918d1..c7318bb 100644
---
a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQServer.java
+++
b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/ActiveMQServer.java
@@ -339,6 +339,8 @@ public interface ActiveMQServer extends ServiceComponent {
PostOffice getPostOffice();
+ void clearAddressCache();
+
QueueFactory getQueueFactory();
ResourceManager getResourceManager();
diff --git
a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java
b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java
index 5855c46..92db847 100644
---
a/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java
+++
b/artemis-server/src/main/java/org/apache/activemq/artemis/core/server/impl/ActiveMQServerImpl.java
@@ -2075,65 +2075,69 @@ public class ActiveMQServerImpl implements
ActiveMQServer {
return;
}
+ try {
+ Binding binding = postOffice.getBinding(queueName);
- addressSettingsRepository.clearCache();
-
- Binding binding = postOffice.getBinding(queueName);
-
- if (binding == null) {
- throw ActiveMQMessageBundle.BUNDLE.noSuchQueue(queueName);
- }
-
- SimpleString address = binding.getAddress();
+ if (binding == null) {
+ throw ActiveMQMessageBundle.BUNDLE.noSuchQueue(queueName);
+ }
- Queue queue = (Queue) binding.getBindable();
+ SimpleString address = binding.getAddress();
- if (hasBrokerQueuePlugins()) {
- callBrokerQueuePlugins(plugin -> plugin.beforeDestroyQueue(queueName,
session, checkConsumerCount,
- removeConsumers, autoDeleteAddress));
- }
+ Queue queue = (Queue) binding.getBindable();
+ if (hasBrokerQueuePlugins()) {
+ callBrokerQueuePlugins(plugin ->
plugin.beforeDestroyQueue(queueName, session, checkConsumerCount,
removeConsumers, autoDeleteAddress));
+ }
- if (session != null) {
+ if (session != null) {
- if (queue.isDurable()) {
- // make sure the user has privileges to delete this queue
- securityStore.check(address, queueName,
CheckType.DELETE_DURABLE_QUEUE, session);
- } else {
- securityStore.check(address, queueName,
CheckType.DELETE_NON_DURABLE_QUEUE, session);
+ if (queue.isDurable()) {
+ // make sure the user has privileges to delete this queue
+ securityStore.check(address, queueName,
CheckType.DELETE_DURABLE_QUEUE, session);
+ } else {
+ securityStore.check(address, queueName,
CheckType.DELETE_NON_DURABLE_QUEUE, session);
+ }
}
- }
- // This check is only valid if checkConsumerCount == true
- if (checkConsumerCount && queue.getConsumerCount() != 0) {
- throw
ActiveMQMessageBundle.BUNDLE.cannotDeleteQueueWithConsumers(queue.getName(),
queueName, binding.getClass().getName());
- }
+ // This check is only valid if checkConsumerCount == true
+ if (checkConsumerCount && queue.getConsumerCount() != 0) {
+ throw
ActiveMQMessageBundle.BUNDLE.cannotDeleteQueueWithConsumers(queue.getName(),
queueName, binding.getClass().getName());
+ }
- // This check is only valid if checkMessageCount == true
- if (checkMessageCount && queue.getAutoDeleteMessageCount() != -1) {
- long messageCount = queue.getMessageCount();
- if (queue.getMessageCount() > queue.getAutoDeleteMessageCount()) {
- throw
ActiveMQMessageBundle.BUNDLE.cannotDeleteQueueWithMessages(queue.getName(),
queueName, messageCount);
+ // This check is only valid if checkMessageCount == true
+ if (checkMessageCount && queue.getAutoDeleteMessageCount() != -1) {
+ long messageCount = queue.getMessageCount();
+ if (queue.getMessageCount() > queue.getAutoDeleteMessageCount()) {
+ throw
ActiveMQMessageBundle.BUNDLE.cannotDeleteQueueWithMessages(queue.getName(),
queueName, messageCount);
+ }
}
- }
- queue.deleteQueue(removeConsumers);
+ queue.deleteQueue(removeConsumers);
- if (hasBrokerQueuePlugins()) {
- callBrokerQueuePlugins(plugin -> plugin.afterDestroyQueue(queue,
address, session, checkConsumerCount,
- removeConsumers, autoDeleteAddress));
- }
- AddressInfo addressInfo = getAddressInfo(address);
+ if (hasBrokerQueuePlugins()) {
+ callBrokerQueuePlugins(plugin -> plugin.afterDestroyQueue(queue,
address, session, checkConsumerCount, removeConsumers, autoDeleteAddress));
+ }
+ AddressInfo addressInfo = getAddressInfo(address);
- if (autoDeleteAddress && postOffice != null && addressInfo != null &&
addressInfo.isAutoCreated() && !isAddressBound(address.toString()) &&
addressSettingsRepository.getMatch(address.toString()).getAutoDeleteAddressesDelay()
== 0) {
- try {
- removeAddressInfo(address, session);
- } catch (ActiveMQDeleteAddressException e) {
- // Could be thrown if the address has bindings or is not deletable.
+ if (autoDeleteAddress && postOffice != null && addressInfo != null &&
addressInfo.isAutoCreated() && !isAddressBound(address.toString()) &&
addressSettingsRepository.getMatch(address.toString()).getAutoDeleteAddressesDelay()
== 0) {
+ try {
+ removeAddressInfo(address, session);
+ } catch (ActiveMQDeleteAddressException e) {
+ // Could be thrown if the address has bindings or is not
deletable.
+ }
}
+
+ callPostQueueDeletionCallbacks(address, queueName);
+ } finally {
+ clearAddressCache();
}
+ }
- callPostQueueDeletionCallbacks(address, queueName);
+ @Override
+ public void clearAddressCache() {
+ securityRepository.clearCache();
+ addressSettingsRepository.clearCache();
}
@Override
diff --git
a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/jms/client/TemporaryDestinationTest.java
b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/jms/client/TemporaryDestinationTest.java
index c08e9c8..c4fe54f 100644
---
a/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/jms/client/TemporaryDestinationTest.java
+++
b/tests/integration-tests/src/test/java/org/apache/activemq/artemis/tests/integration/jms/client/TemporaryDestinationTest.java
@@ -24,12 +24,16 @@ import javax.jms.Session;
import javax.jms.TemporaryQueue;
import javax.jms.TemporaryTopic;
import javax.jms.TextMessage;
+import java.util.HashSet;
+import java.util.Set;
import org.apache.activemq.artemis.api.core.SimpleString;
+import org.apache.activemq.artemis.core.security.Role;
import org.apache.activemq.artemis.core.server.ServerSession;
import org.apache.activemq.artemis.core.server.impl.ServerSessionImpl;
import org.apache.activemq.artemis.core.settings.impl.AddressSettings;
import org.apache.activemq.artemis.jms.client.ActiveMQConnection;
+import
org.apache.activemq.artemis.spi.core.security.ActiveMQJAASSecurityManager;
import org.apache.activemq.artemis.tests.util.JMSTestBase;
import org.junit.Before;
import org.junit.Test;
@@ -222,4 +226,30 @@ public class TemporaryDestinationTest extends JMSTestBase {
}
}
}
+
+ @Test
+ public void testForSecurityCacheLeak() throws Exception {
+ server.getSecurityStore().setSecurityEnabled(true);
+ ActiveMQJAASSecurityManager securityManager =
(ActiveMQJAASSecurityManager) server.getSecurityManager();
+ securityManager.getConfiguration().addUser("IDo", "Exist");
+ securityManager.getConfiguration().addRole("IDo", "myrole");
+ Role myRole = new Role("myrole", true, true, true, true, true, true,
true, true, true, true);
+ Set<Role> anySet = new HashSet<>();
+ anySet.add(myRole);
+ server.getSecurityRepository().addMatch("#", anySet);
+
+ try {
+ conn = addConnection(cf.createConnection("IDo", "Exist"));
+ Session s = conn.createSession(false, Session.AUTO_ACKNOWLEDGE);
+ for (int i = 0; i < 10; i++) {
+ TemporaryQueue temporaryQueue = s.createTemporaryQueue();
+ temporaryQueue.delete();
+ }
+ assertEquals(0, server.getSecurityRepository().getCacheSize());
+ } finally {
+ if (conn != null) {
+ conn.close();
+ }
+ }
+ }
}
