activemq-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Arthur Naseef (Commented) (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (AMQ-3457) temp destinations should only be deleted once all users of a pooled connection call close
Date Thu, 19 Jan 2012 16:13:42 GMT

    [ https://issues.apache.org/jira/browse/AMQ-3457?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13189171#comment-13189171
] 

Arthur Naseef commented on AMQ-3457:
------------------------------------

Below is my understanding.  I'll work on a JUnit case, and send a proposal that I believe
fixes the issue, a little later today.

* The PooledConnection no longer cleans up the temp destinations itself
* ConnectionPool removes the temp destinations after all references to the ConnectionPool
are dropped (meaning that the underlying ActiveMQConnection is also no longer used)
* PooledConnection gets its ConnectionPool from the PooledConnectionFactory when instantiated.

* PooledConnectionFactory keeps a cache of ConnectionPool objects in a hash-map keyed on the
user-name + passsword.
** On createConnection(), if the ConnectionPool (with the matching username + password) is
already in the cache, it is passed to the newly created PooledConnection.
** If it's not already in the cache, a new one is added to the cache and used for the newly
created PooledConnection.

Here are the changes to ConnectionPool and PooledConnection for this bug that I believe back
this up.

{code}
Index: trunk/activemq-pool/src/main/java/org/apache/activemq/pool/ConnectionPool.java
===================================================================
diff -u -N -r1071256 -r1158694
--- trunk/activemq-pool/src/main/java/org/apache/activemq/pool/ConnectionPool.java	(.../ConnectionPool.java)
(revision 1071256)
+++ trunk/activemq-pool/src/main/java/org/apache/activemq/pool/ConnectionPool.java	(.../ConnectionPool.java)
(revision 1158694)
@@ -144,6 +144,12 @@
         lastUsed = System.currentTimeMillis();
         if (referenceCount == 0) {
             expiredCheck();
+            
+            // only clean up temp destinations when all users 
+            // of this connection have called close
+            if (getConnection() != null) {
+                getConnection().cleanUpTempDestinations();
+            }
         }
     }
 
Index: trunk/activemq-pool/src/main/java/org/apache/activemq/pool/PooledConnection.java
===================================================================
diff -u -N -r1142267 -r1158694
--- trunk/activemq-pool/src/main/java/org/apache/activemq/pool/PooledConnection.java	(.../PooledConnection.java)
(revision 1142267)
+++ trunk/activemq-pool/src/main/java/org/apache/activemq/pool/PooledConnection.java	(.../PooledConnection.java)
(revision 1158694)
@@ -16,9 +16,6 @@
  */
 package org.apache.activemq.pool;
 
-import java.util.Iterator;
-import java.util.concurrent.ConcurrentHashMap;
-
 import javax.jms.Connection;
 import javax.jms.ConnectionConsumer;
 import javax.jms.ConnectionMetaData;
@@ -39,7 +36,6 @@
 import org.apache.activemq.AlreadyClosedException;
 import org.apache.activemq.EnhancedConnection;
 import org.apache.activemq.advisory.DestinationSource;
-import org.apache.activemq.command.ActiveMQTempDestination;
 
 /**
  * Represents a proxy {@link Connection} which is-a {@link TopicConnection} and
@@ -73,9 +69,6 @@
     public void close() throws JMSException {
         if (this.pool != null) {
             this.pool.decrementReferenceCount();
-            if (this.pool.getConnection() != null) {
-                this.pool.getConnection().cleanUpTempDestinations();
-            }
             this.pool = null;
         }
     }
{code}

By the way, for a JUnit test - do you have any recommendation on testing whether a temporary
destination has been deleted?  My plan is to produce to it from another connection and expect
an exception.
                
> temp destinations should only be deleted once all users of a pooled connection call close
> -----------------------------------------------------------------------------------------
>
>                 Key: AMQ-3457
>                 URL: https://issues.apache.org/jira/browse/AMQ-3457
>             Project: ActiveMQ
>          Issue Type: Bug
>    Affects Versions: 5.x
>         Environment: 5.6-SNAPSHOT
>            Reporter: Jonathan Anstey
>            Assignee: Jonathan Anstey
>             Fix For: 5.6.0
>
>         Attachments: amqpool.diff
>
>
> AMQ-2349 added some code to clean up temp destinations once close() is called on a pooled
connection. This caused pretty much all the JMS request-reply tests to fail in Camel with
"javax.jms.InvalidDestinationException: Cannot publish to a deleted Destination" :) 
> The problem is that with a pooled connection, multiple users can be using a Connection
at the same time (a reference count is kept of how many there are) so if once calls close()
the temp destinations of several others could be deleted while they are still using them.
I think the correct behavior would be to only delete the temp destinations when all connection
users call close() (i.e. when the reference count becomes zero).
> Attaching a proposed fix for this shortly for review.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Mime
View raw message