You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cxf.apache.org by GitBox <gi...@apache.org> on 2021/07/13 15:47:03 UTC
[GitHub] [cxf] valentin-matignon opened a new pull request #826: Delete temporary queue when it is used
valentin-matignon opened a new pull request #826:
URL: https://github.com/apache/cxf/pull/826
Temporary queues are never deleted when the are used.
This patch provides fixes for this behavior.
--
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: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] valentin-matignon commented on a change in pull request #826: Delete temporary queue when it is used
Posted by GitBox <gi...@apache.org>.
valentin-matignon commented on a change in pull request #826:
URL: https://github.com/apache/cxf/pull/826#discussion_r694031785
##########
File path: rt/transports/jms/src/main/java/org/apache/cxf/transport/jms/JMSConduit.java
##########
@@ -282,6 +283,9 @@ private void sendAndReceiveMessage(final Exchange exchange, final Object request
jmsConfig.getReceiveTimeout(),
jmsConfig.isPubSubNoLocal(),
exchange);
+ if (replyDestination instanceof TemporaryQueue) {
Review comment:
Hi @reta, thank you for your answer and sorry to respond so late !
I agree with you, `JMSConfiguration` should delete temporary queues.
As you advised, in my last commit i delete temporary queues in the `resetCachedReplyDestination` method of the `JMSConfiguration` class.
However, `resetCachedReplyDestination` is called in two places places in the `JMSConduit` class :
- in a `ExceptionListener` in the `trySetExListener` method
- in the `sendExchange` method when a `JMSException` occurs
So i had to add the `resetCachedReplyDestination` in the `close` method of `JMSConduit` class to be sure that temporary queues are deleted every time they are used.
I didn't find a proper way to add one test case in unit tests, if you have some suggestions i could add it in my PR.
--
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: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] valentin-matignon commented on a change in pull request #826: Delete temporary queue when it is used
Posted by GitBox <gi...@apache.org>.
valentin-matignon commented on a change in pull request #826:
URL: https://github.com/apache/cxf/pull/826#discussion_r699353850
##########
File path: rt/transports/jms/src/main/java/org/apache/cxf/transport/jms/JMSConduit.java
##########
@@ -510,6 +510,7 @@ private synchronized void shutdownListeners() {
}
}
public synchronized void close() {
+ jmsConfig.resetCachedReplyDestination();
Review comment:
I moved it.
--
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: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] valentin-matignon commented on a change in pull request #826: Delete temporary queue when it is used
Posted by GitBox <gi...@apache.org>.
valentin-matignon commented on a change in pull request #826:
URL: https://github.com/apache/cxf/pull/826#discussion_r699353850
##########
File path: rt/transports/jms/src/main/java/org/apache/cxf/transport/jms/JMSConduit.java
##########
@@ -510,6 +510,7 @@ private synchronized void shutdownListeners() {
}
}
public synchronized void close() {
+ jmsConfig.resetCachedReplyDestination();
Review comment:
I moved it.
--
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: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] valentin-matignon commented on a change in pull request #826: Delete temporary queue when it is used
Posted by GitBox <gi...@apache.org>.
valentin-matignon commented on a change in pull request #826:
URL: https://github.com/apache/cxf/pull/826#discussion_r700862858
##########
File path: rt/transports/jms/src/main/java/org/apache/cxf/transport/jms/JMSConfiguration.java
##########
@@ -19,22 +19,29 @@
package org.apache.cxf.transport.jms;
import java.util.Properties;
+import java.util.logging.Level;
+import java.util.logging.Logger;
import javax.jms.ConnectionFactory;
import javax.jms.Destination;
import javax.jms.JMSException;
import javax.jms.Message;
import javax.jms.Session;
+import javax.jms.TemporaryQueue;
import javax.naming.NamingException;
import javax.transaction.TransactionManager;
import org.apache.cxf.common.injection.NoJSR250Annotations;
+import org.apache.cxf.common.logging.LogUtils;
import org.apache.cxf.transport.jms.util.DestinationResolver;
import org.apache.cxf.transport.jms.util.JMSDestinationResolver;
import org.apache.cxf.transport.jms.util.JndiHelper;
@NoJSR250Annotations
public class JMSConfiguration {
+
+ private static final Logger LOG = LogUtils.getL7dLogger(JMSConfiguration.class);
Review comment:
No problem, done.
--
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: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] reta commented on a change in pull request #826: Delete temporary queue when it is used
Posted by GitBox <gi...@apache.org>.
reta commented on a change in pull request #826:
URL: https://github.com/apache/cxf/pull/826#discussion_r698815702
##########
File path: rt/transports/jms/src/main/java/org/apache/cxf/transport/jms/JMSConduit.java
##########
@@ -510,6 +510,7 @@ private synchronized void shutdownListeners() {
}
}
public synchronized void close() {
+ jmsConfig.resetCachedReplyDestination();
Review comment:
I believe we should call it after the `shutdownListeners()` (because we still have listeners on the queue being deleted) but before `ResourceCloser.close(connection)`.
--
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: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] reta commented on a change in pull request #826: Delete temporary queue when it is used
Posted by GitBox <gi...@apache.org>.
reta commented on a change in pull request #826:
URL: https://github.com/apache/cxf/pull/826#discussion_r700642575
##########
File path: rt/transports/jms/src/main/java/org/apache/cxf/transport/jms/JMSConduit.java
##########
@@ -511,6 +511,7 @@ private synchronized void shutdownListeners() {
}
public synchronized void close() {
shutdownListeners();
+ jmsConfig.resetCachedReplyDestination();
Review comment:
One similar modification is needed in `sendExchange` method, right now the connection is closed before the `jmsConfig.resetCachedReplyDestination();`, so there is no chance to delete temporary query. The correct sequence should be:
```
jmsConfig.resetCachedReplyDestination();
ResourceCloser.close(connection);
this.connection = null;
```
Thank you.
--
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: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] reta commented on a change in pull request #826: Delete temporary queue when it is used
Posted by GitBox <gi...@apache.org>.
reta commented on a change in pull request #826:
URL: https://github.com/apache/cxf/pull/826#discussion_r696040338
##########
File path: rt/transports/jms/src/main/java/org/apache/cxf/transport/jms/JMSConduit.java
##########
@@ -282,6 +283,9 @@ private void sendAndReceiveMessage(final Exchange exchange, final Object request
jmsConfig.getReceiveTimeout(),
jmsConfig.isPubSubNoLocal(),
exchange);
+ if (replyDestination instanceof TemporaryQueue) {
Review comment:
Thank you @valentin-matignon, the change looks good to me, I will surely help you out with test case(s), will get back to you shortly.
--
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: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] valentin-matignon commented on a change in pull request #826: Delete temporary queue when it is used
Posted by GitBox <gi...@apache.org>.
valentin-matignon commented on a change in pull request #826:
URL: https://github.com/apache/cxf/pull/826#discussion_r699353850
##########
File path: rt/transports/jms/src/main/java/org/apache/cxf/transport/jms/JMSConduit.java
##########
@@ -510,6 +510,7 @@ private synchronized void shutdownListeners() {
}
}
public synchronized void close() {
+ jmsConfig.resetCachedReplyDestination();
Review comment:
I moved it.
--
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: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] reta commented on a change in pull request #826: Delete temporary queue when it is used
Posted by GitBox <gi...@apache.org>.
reta commented on a change in pull request #826:
URL: https://github.com/apache/cxf/pull/826#discussion_r677899019
##########
File path: rt/transports/jms/src/main/java/org/apache/cxf/transport/jms/JMSConduit.java
##########
@@ -282,6 +283,9 @@ private void sendAndReceiveMessage(final Exchange exchange, final Object request
jmsConfig.getReceiveTimeout(),
jmsConfig.isPubSubNoLocal(),
exchange);
+ if (replyDestination instanceof TemporaryQueue) {
Review comment:
Thanks for the patch @valentin-matignon , I believe the this is not the right place to delete the temporary queue.
- `JMSConfiguration` is the class that creates temporary queue (and caches it)
- `JMSConfiguration` has the method `resetCachedReplyDestination` which looks like the right place to delete the temporary queue (it caches the destination in `replyToDestinationDest`)
- the `JMSConduit` should be likely calling `JMSConfiguration::resetCachedReplyDestination()` on shutdown
- it would be great to have at least one test case for the issue
What do you think? Thank you.
--
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: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] reta commented on a change in pull request #826: Delete temporary queue when it is used
Posted by GitBox <gi...@apache.org>.
reta commented on a change in pull request #826:
URL: https://github.com/apache/cxf/pull/826#discussion_r698815702
##########
File path: rt/transports/jms/src/main/java/org/apache/cxf/transport/jms/JMSConduit.java
##########
@@ -510,6 +510,7 @@ private synchronized void shutdownListeners() {
}
}
public synchronized void close() {
+ jmsConfig.resetCachedReplyDestination();
Review comment:
I believe we should call it after the `shutdownListeners()` (because we still have listeners on the queue being deleted) but before `ResourceCloser.close(connection)`.
--
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: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] reta commented on pull request #826: Delete temporary queue when it is used
Posted by GitBox <gi...@apache.org>.
reta commented on pull request #826:
URL: https://github.com/apache/cxf/pull/826#issuecomment-909705805
--
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: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] reta edited a comment on pull request #826: Delete temporary queue when it is used
Posted by GitBox <gi...@apache.org>.
reta edited a comment on pull request #826:
URL: https://github.com/apache/cxf/pull/826#issuecomment-909707257
@valentin-matignon do you mind if I push one test case to your pull request? Thank you.
--
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: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] reta commented on pull request #826: Delete temporary queue when it is used
Posted by GitBox <gi...@apache.org>.
reta commented on pull request #826:
URL: https://github.com/apache/cxf/pull/826#issuecomment-910906820
Issue: https://issues.apache.org/jira/browse/CXF-8591
--
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: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] valentin-matignon commented on pull request #826: Delete temporary queue when it is used
Posted by GitBox <gi...@apache.org>.
valentin-matignon commented on pull request #826:
URL: https://github.com/apache/cxf/pull/826#issuecomment-909285149
@reta, i'm using a TIBCO broker and the **_com.tibco:tibjms:8.3.0_**, **_javax.jms:javax.jms-api:2.0.1_** dependencies in my program.
Which tests are you talking about ?
- I tested `testRequestQueueResponseTempQueue` and `testRequestTopicResponseTempQueue` tests of `RequestResponseTest` class and the `delete` method of the associated temporary queue (`ActiveMQTempQueue` in this case) **is never called** because it is not called by the `close` method of the associated connection (`ActiveMQConnection` in this case)
- I tested `testTempQueueIssue` test of `PooledConnectionTempQueueTest` class and the `delete` method of the associated temporary queue (`ActiveMQTempQueue` in this case) **is called** because it is called by the `close` method of the associated connection (`PooledConnection` in this case)
Have you noticed the same or am I wrong?
--
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: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] valentin-matignon commented on a change in pull request #826: Delete temporary queue when it is used
Posted by GitBox <gi...@apache.org>.
valentin-matignon commented on a change in pull request #826:
URL: https://github.com/apache/cxf/pull/826#discussion_r700863517
##########
File path: rt/transports/jms/src/main/java/org/apache/cxf/transport/jms/JMSConduit.java
##########
@@ -511,6 +511,7 @@ private synchronized void shutdownListeners() {
}
public synchronized void close() {
shutdownListeners();
+ jmsConfig.resetCachedReplyDestination();
Review comment:
You're right, done.
--
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: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] reta edited a comment on pull request #826: Delete temporary queue when it is used
Posted by GitBox <gi...@apache.org>.
reta edited a comment on pull request #826:
URL: https://github.com/apache/cxf/pull/826#issuecomment-909707257
@valentin-matignon do you mind if I push one test case to your pull request? Thank you.
--
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: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] valentin-matignon commented on pull request #826: Delete temporary queue when it is used
Posted by GitBox <gi...@apache.org>.
valentin-matignon commented on pull request #826:
URL: https://github.com/apache/cxf/pull/826#issuecomment-909285149
--
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: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] reta commented on pull request #826: Delete temporary queue when it is used
Posted by GitBox <gi...@apache.org>.
reta commented on pull request #826:
URL: https://github.com/apache/cxf/pull/826#issuecomment-908742856
@valentin-matignon I was testing the changes and it does not seem like this PR is really needed. The JMS spec [1] says
> TemporaryQueue createTemporaryQueue() throws JMSException
>
> Creates a TemporaryQueue object. Its lifetime will be that of the Connection unless it is deleted earlier.
And indeed their lifetime is bound to the connection (tests confirm that): connection closes, queue is gone. So my question to you is: which message broker are you using that is keeping the temporary queues upon connection closing?
[1] https://docs.oracle.com/javaee/7/api/javax/jms/Session.html#createTemporaryQueue--
--
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: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] reta commented on a change in pull request #826: Delete temporary queue when it is used
Posted by GitBox <gi...@apache.org>.
reta commented on a change in pull request #826:
URL: https://github.com/apache/cxf/pull/826#discussion_r700642575
##########
File path: rt/transports/jms/src/main/java/org/apache/cxf/transport/jms/JMSConduit.java
##########
@@ -511,6 +511,7 @@ private synchronized void shutdownListeners() {
}
public synchronized void close() {
shutdownListeners();
+ jmsConfig.resetCachedReplyDestination();
Review comment:
One similar modification is need in `sendExchange` method, right now the connection is closed before the `jmsConfig.resetCachedReplyDestination();`, so there is no chance to delete temporary query. The correct sequence should be:
```
jmsConfig.resetCachedReplyDestination();
ResourceCloser.close(connection);
this.connection = null;
```
##########
File path: rt/transports/jms/src/main/java/org/apache/cxf/transport/jms/JMSConduit.java
##########
@@ -511,6 +511,7 @@ private synchronized void shutdownListeners() {
}
public synchronized void close() {
shutdownListeners();
+ jmsConfig.resetCachedReplyDestination();
Review comment:
One similar modification is need in `sendExchange` method, right now the connection is closed before the `jmsConfig.resetCachedReplyDestination();`, so there is no chance to delete temporary query. The correct sequence should be:
```
jmsConfig.resetCachedReplyDestination();
ResourceCloser.close(connection);
this.connection = null;
```
Thank you.
--
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: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] reta commented on pull request #826: Delete temporary queue when it is used
Posted by GitBox <gi...@apache.org>.
reta commented on pull request #826:
URL: https://github.com/apache/cxf/pull/826#issuecomment-909707257
@valentin-matignon do you mind if I push one test case your pull request? thank you.
--
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: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] reta commented on pull request #826: Delete temporary queue when it is used
Posted by GitBox <gi...@apache.org>.
reta commented on pull request #826:
URL: https://github.com/apache/cxf/pull/826#issuecomment-910897349
>
>
> @reta no problem, you can do it.
Sadly, don't have permissions to push to your branch, I will suggest 2 changes and will add the test case after merge.
--
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: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] reta commented on a change in pull request #826: Delete temporary queue when it is used
Posted by GitBox <gi...@apache.org>.
reta commented on a change in pull request #826:
URL: https://github.com/apache/cxf/pull/826#discussion_r700641662
##########
File path: rt/transports/jms/src/main/java/org/apache/cxf/transport/jms/JMSConfiguration.java
##########
@@ -19,22 +19,29 @@
package org.apache.cxf.transport.jms;
import java.util.Properties;
+import java.util.logging.Level;
+import java.util.logging.Logger;
import javax.jms.ConnectionFactory;
import javax.jms.Destination;
import javax.jms.JMSException;
import javax.jms.Message;
import javax.jms.Session;
+import javax.jms.TemporaryQueue;
import javax.naming.NamingException;
import javax.transaction.TransactionManager;
import org.apache.cxf.common.injection.NoJSR250Annotations;
+import org.apache.cxf.common.logging.LogUtils;
import org.apache.cxf.transport.jms.util.DestinationResolver;
import org.apache.cxf.transport.jms.util.JMSDestinationResolver;
import org.apache.cxf.transport.jms.util.JndiHelper;
@NoJSR250Annotations
public class JMSConfiguration {
+
+ private static final Logger LOG = LogUtils.getL7dLogger(JMSConfiguration.class);
Review comment:
Could you please move `LOG` after `DEFAULT_VALUE`, (the `Checkstyle` will complain)
--
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: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] reta commented on a change in pull request #826: Delete temporary queue when it is used
Posted by GitBox <gi...@apache.org>.
reta commented on a change in pull request #826:
URL: https://github.com/apache/cxf/pull/826#discussion_r677899019
##########
File path: rt/transports/jms/src/main/java/org/apache/cxf/transport/jms/JMSConduit.java
##########
@@ -282,6 +283,9 @@ private void sendAndReceiveMessage(final Exchange exchange, final Object request
jmsConfig.getReceiveTimeout(),
jmsConfig.isPubSubNoLocal(),
exchange);
+ if (replyDestination instanceof TemporaryQueue) {
Review comment:
Thanks for the patch @valentin-matignon , I believe the this is not the right place to delete the temporary queue.
- `JMSConfiguration` is the class that create temporary queue (and caches it)
- `JMSConfiguration` has the method `resetCachedReplyDestination` which looks like the right place to delete the temporary queue (it caches the destination in `replyToDestinationDest`)
- the `JMSConduit` should be likely calling `JMSConfiguration::resetCachedReplyDestination()` on shutdown
- it would be great to have at least one test case for the issue
What do you think? Thank you.
--
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: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] valentin-matignon commented on pull request #826: Delete temporary queue when it is used
Posted by GitBox <gi...@apache.org>.
valentin-matignon commented on pull request #826:
URL: https://github.com/apache/cxf/pull/826#issuecomment-911590525
> Thank you @valentin-matignon !
Thanks to you for the code review @reta 😄 !
--
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: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] reta commented on pull request #826: Delete temporary queue when it is used
Posted by GitBox <gi...@apache.org>.
reta commented on pull request #826:
URL: https://github.com/apache/cxf/pull/826#issuecomment-908742856
@valentin-matignon I was testing the changes and it does not seem like this PR is really needed. The JMS spec [1] says
> TemporaryQueue createTemporaryQueue() throws JMSException
>
> Creates a TemporaryQueue object. Its lifetime will be that of the Connection unless it is deleted earlier.
And indeed their lifetime is bound to the connection (tests confirm that): connection closes, queue is gone. So my question to you is: which message broker are you using that is keeping the temporary queues upon connection closing?
[1] https://docs.oracle.com/javaee/7/api/javax/jms/Session.html#createTemporaryQueue--
--
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: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] valentin-matignon commented on pull request #826: Delete temporary queue when it is used
Posted by GitBox <gi...@apache.org>.
valentin-matignon commented on pull request #826:
URL: https://github.com/apache/cxf/pull/826#issuecomment-910003875
@reta no problem, you can do it.
--
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: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] valentin-matignon commented on pull request #826: Delete temporary queue when it is used
Posted by GitBox <gi...@apache.org>.
valentin-matignon commented on pull request #826:
URL: https://github.com/apache/cxf/pull/826#issuecomment-909285149
@reta, i'm using a TIBCO broker and the **_com.tibco:tibjms:8.3.0_**, **_javax.jms:javax.jms-api:2.0.1_** dependencies in my program.
Which tests are you talking about ?
- I tested `testRequestQueueResponseTempQueue` and `testRequestTopicResponseTempQueue` tests of `RequestResponseTest` class and the `delete` method of the associated temporary queue (`ActiveMQTempQueue` in this case) **is never called** because it is not called by the `close` method of the associated connection (`ActiveMQConnection` in this case)
- I tested `testTempQueueIssue` test of `PooledConnectionTempQueueTest` class and the `delete` method of the associated temporary queue (`ActiveMQTempQueue` in this case) **is called** because it is called by the `close` method of the associated connection (`PooledConnection` in this case)
Have you noticed the same or am I wrong?
--
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: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] reta commented on pull request #826: Delete temporary queue when it is used
Posted by GitBox <gi...@apache.org>.
reta commented on pull request #826:
URL: https://github.com/apache/cxf/pull/826#issuecomment-909705805
@valentin-matignon you are right, at least in case of ActiveMQ, the pooled connection do cleanup temporary queue, but non-pooled - not. My apologies for confusion, I was checking ActiveMQ's `DestinationSource` and it was properly reporting the removal of the temporary queues (but it seems like - not physical removal).
--
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: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [cxf] reta merged pull request #826: Delete temporary queue when it is used
Posted by GitBox <gi...@apache.org>.
reta merged pull request #826:
URL: https://github.com/apache/cxf/pull/826
--
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: dev-unsubscribe@cxf.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org