[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/ConnectionPool.java

@@ -125,6 +125,7 @@ public void start() throws JMSException {
                 connection.start();
             } catch (JMSException e) {
                 started.set(false);
+                setHasExpired(true);
                 if (isReconnectOnException()) {
                     close();
                 }

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

@@ -129,12 +129,7 @@ public void destroyObject(ConnectionKey connectionKey, PooledObject<ConnectionPo
                     @Override
                     public boolean validateObject(ConnectionKey connectionKey, PooledObject<ConnectionPool> pooledObject) {
                         ConnectionPool connection = pooledObject.getObject();
-                        if (connection == null || connection.getConnection() == null) {
-                            LOG.trace("Connection has been closed and will be destroyed: {}", connection);
-                            return false;
-                        }
-
-                        if (connection.expiredCheck()) {
+                        if (connection != null && 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();
         }
     }
@@ -136,13 +134,15 @@ public void testFailureGetsNewConnectionOnRetry() throws Exception {
             @Override
             public boolean isSatisified() throws Exception {
                 try (final PooledConnection newConnection = (PooledConnection) pooledConnFact.createConnection("invalid", "credentials")) {
-                    return connection1.getConnection() != newConnection.getConnection();
+                    return connection1.pool != ((PooledConnection)newConnection).pool;
+                } catch (Exception e) {
+                    return false;
                 }
             }
         }));
 
         final PooledConnection connection2 = (PooledConnection) pooledConnFact.createConnection("invalid", "credentials");
-        assertNotSame(connection1.getConnection(), connection2.getConnection());
+        assertNotSame(connection1.pool, connection2.pool);
 
         try {
             connection2.start();
@@ -235,13 +235,15 @@ public void testFailoverWithInvalidCredentials() throws Exception {
             @Override
             public boolean isSatisified() throws Exception {
                 try (final PooledConnection newConnection = (PooledConnection) pooledConnFact.createConnection("invalid", "credentials")) {
-                    return connection1.getConnection() != newConnection.getConnection();
+                    return connection1.pool != ((PooledConnection)newConnection).pool;
+                } catch (Exception e) {
+                    return false;
                 }
             }
         }));
 
         final PooledConnection connection2 = (PooledConnection) pooledConnFact.createConnection("invalid", "credentials");
-        assertNotSame(connection1.getConnection(), connection2.getConnection());
+        assertNotSame(connection1.pool, connection2.pool);
 
         try {
             connection2.start();
@@ -412,4 +414,4 @@ protected BrokerPlugin configureAuthorization() throws Exception {
 
         return authorizationPlugin;
     }
-}
+}