cshannon commented on code in PR #1484:
URL: https://github.com/apache/activemq/pull/1484#discussion_r2727739331
##########
activemq-broker/src/main/java/org/apache/activemq/broker/region/AbstractRegion.java:
##########
@@ -260,16 +260,21 @@ protected List<Subscription>
addSubscriptionsForDestination(ConnectionContext co
}
@Override
- public void removeDestination(ConnectionContext context,
ActiveMQDestination destination, long timeout)
- throws Exception {
-
+ public void removeDestination(ConnectionContext context,
ActiveMQDestination destination, long timeout) throws Exception {
// No timeout.. then try to shut down right way, fails if there are
// current subscribers.
if (timeout == 0) {
for (Iterator<Subscription> iter =
subscriptions.values().iterator(); iter.hasNext();) {
Subscription sub = iter.next();
if (sub.matches(destination) ) {
- throw new JMSException("Destination: " + destination + "
still has an active subscription: " + sub);
+ if(sub.isWildcard()) {
+ var dest = destinations.get(destination);
+ if(dest != null &&
dest.isGcWithOnlyWildcardConsumers()) {
Review Comment:
There still seems to be a missing test case because this logic is broken,
Intellij flagged it. This continue does nothing as it's in a loop and this will
not throw the exception if isGcWithOnlyWildcardConsumers() is false.
This should likely just be rewritten as something like and include a test
that would catch a similar mistake in the future.
```java
if (sub.matches(destination) ) {
var dest = destinations.get(destination);
if(!sub.isWildcard() || dest == null ||
!dest.isGcWithOnlyWildcardConsumers()) {
throw new JMSException("Destination: " + destination + " still has
an active subscription: " + sub);
}
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
For further information, visit: https://activemq.apache.org/contact
