[AMQ-9820]: closed connections leaking into the pool when reconnectOnException is used

Author: jeanouii

activemq-jms-pool/src/main/java/org/apache/activemq/jms/pool/PooledConnectionFactory.java

@@ -129,7 +129,12 @@ public void destroyObject(ConnectionKey connectionKey, PooledObject<ConnectionPo
                     @Override
                     public boolean validateObject(ConnectionKey connectionKey, PooledObject<ConnectionPool> pooledObject) {
                         ConnectionPool connection = pooledObject.getObject();
-                        if (connection != null && connection.expiredCheck()) {
+                        if (connection == null || connection.getConnection() == null) {
+                            LOG.trace("Connection has been closed and will be destroyed: {}", connection);
+                            return false;
+                        }
+
+                        if (connection.expiredCheck()) {
                             LOG.trace("Connection has expired: {} and will be destroyed", connection);
                             return false;
                         }

activemq-jms-pool/src/test/java/org/apache/activemq/jms/pool/PooledConnectionSecurityExceptionTest.java

@@ -16,21 +16,13 @@
  */
 package org.apache.activemq.jms.pool;
 
-import static org.junit.Assert.assertNotSame;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
-
-import java.util.ArrayList;
-import java.util.List;
-
 import jakarta.jms.Connection;
 import jakarta.jms.ExceptionListener;
 import jakarta.jms.JMSException;
 import jakarta.jms.JMSSecurityException;
 import jakarta.jms.MessageProducer;
 import jakarta.jms.Queue;
 import jakarta.jms.Session;
-
 import org.apache.activemq.ActiveMQConnectionFactory;
 import org.apache.activemq.broker.BrokerPlugin;
 import org.apache.activemq.broker.BrokerService;
@@ -50,6 +42,13 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import java.util.ArrayList;
+import java.util.List;
+
+import static org.junit.Assert.assertNotSame;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
+
 /**
  * Test Pooled connections ability to handle security exceptions
  */
@@ -112,7 +111,6 @@ public void onException(JMSException exception) {
     @Test
     public void testFailureGetsNewConnectionOnRetryLooped() throws Exception {
         for (int i = 0; i < 10; ++i) {
-            LOG.info("Iteration: {}", i);
             testFailureGetsNewConnectionOnRetry();
         }
     }
@@ -135,11 +133,8 @@ public void testFailureGetsNewConnectionOnRetry() throws Exception {
 
             @Override
             public boolean isSatisified() throws Exception {
-                PooledConnection newConnection = (PooledConnection) pooledConnFact.createConnection("invalid", "credentials");
-                try {
+                try (final PooledConnection newConnection = (PooledConnection) pooledConnFact.createConnection("invalid", "credentials")) {
                     return connection1.getConnection() != newConnection.getConnection();
-                } finally {
-                    newConnection.close();
                 }
             }
         }));
@@ -237,8 +232,9 @@ public void testFailoverWithInvalidCredentials() throws Exception {
 
             @Override
             public boolean isSatisified() throws Exception {
-                return connection1.getConnection() !=
-                          ((PooledConnection) pooledConnFact.createConnection("invalid", "credentials")).getConnection();
+                try (final PooledConnection newConnection = (PooledConnection) pooledConnFact.createConnection("invalid", "credentials")) {
+                    return connection1.getConnection() != newConnection.getConnection();
+                }
             }
         }));