You are viewing a plain text version of this content. The canonical link for it is here.
Posted to gitbox@activemq.apache.org by "jeanouii (via GitHub)" <gi...@apache.org> on 2025/12/17 14:16:14 UTC
[PR] [AMQ-9820]: closed connections leaking into the pool when reconnectOnException is used [activemq]
jeanouii opened a new pull request, #1564:
URL: https://github.com/apache/activemq/pull/1564
Possible bug revealed by PooledConnectionSecurityExceptionTest on faster environments (Github Actions nodes for example).
The PR fixes the test itself leaking connections during the waitFor condition.
But it also fixes the connection pool itself because when reconnectOnException is used, commons-pool will close the underlying connection in order to reconnect. I've updated the validateObject() to consider the underlying connection state in order to decide if a connection is valid or not.
I'm not quite sure my hypothesis is correct or not and therefore if the fix is relevant.
```
[INFO] Running
org.apache.activemq.jms.pool.PooledConnectionSecurityExceptionTest
Error: Tests run: 8, Failures: 0, Errors: 1, Skipped: 0, Time elapsed: 0.173 s <<< FAILURE! -- in org.apache.activemq.jms.pool.PooledConnectionSecurityExceptionTest
Error: org.apache.activemq.jms.pool.PooledConnectionSecurityExceptionTest.testFailureGetsNewConnectionOnRetryLooped -- Time elapsed: 0.056 s <<< ERROR!
jakarta.jms.JMSException: Disposed due to prior exception
at org.apache.activemq.util.JMSExceptionSupport.create(JMSExceptionSupport.java:67)
at org.apache.activemq.ActiveMQConnection.syncSendPacket(ActiveMQConnection.java:1478)
at org.apache.activemq.ActiveMQConnection.ensureConnectionInfoSent(ActiveMQConnection.java:1561)
at org.apache.activemq.ActiveMQConnection.start(ActiveMQConnection.java:569)
at org.apache.activemq.jms.pool.ConnectionPool.start(ConnectionPool.java:125)
at org.apache.activemq.jms.pool.PooledConnection.start(PooledConnection.java:95)
at org.apache.activemq.jms.pool.PooledConnectionSecurityExceptionTest.testFailureGetsNewConnectionOnRetry(PooledConnectionSecurityExceptionTest.java:127)
at org.apache.activemq.jms.pool.PooledConnectionSecurityExceptionTest.testFailureGetsNewConnectionOnRetryLooped(PooledConnectionSecurityExceptionTest.java:116)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
at java.base/java.lang.reflect.Method.invoke(Method.java:569)
at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
at org.junit.rules.TestWatcher$1.evaluate(TestWatcher.java:61)
at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366)
at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:63)
at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331)
at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79)
at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329)
at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66)
at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293)
at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
at org.junit.runners.ParentRunner.run(ParentRunner.java:413)
at org.apache.maven.surefire.junit4.JUnit4Provider.execute(JUnit4Provider.java:316)
at org.apache.maven.surefire.junit4.JUnit4Provider.executeWithRerun(JUnit4Provider.java:240)
at org.apache.maven.surefire.junit4.JUnit4Provider.executeTestSet(JUnit4Provider.java:214)
at org.apache.maven.surefire.junit4.JUnit4Provider.invoke(JUnit4Provider.java:155)
at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:385)
at org.apache.maven.surefire.booter.ForkedBooter.execute(ForkedBooter.java:162)
at org.apache.maven.surefire.booter.ForkedBooter.run(ForkedBooter.java:507)
at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:495)
Caused by: org.apache.activemq.transport.TransportDisposedIOException: Disposed due to prior exception
at org.apache.activemq.transport.ResponseCorrelator.onException(ResponseCorrelator.java:125)
at org.apache.activemq.transport.TransportFilter.onException(TransportFilter.java:114)
at org.apache.activemq.transport.TransportFilter.onException(TransportFilter.java:114)
at org.apache.activemq.transport.WireFormatNegotiator.onException(WireFormatNegotiator.java:173)
at org.apache.activemq.transport.AbstractInactivityMonitor.onException(AbstractInactivityMonitor.java:349)
at org.apache.activemq.transport.TransportSupport.onException(TransportSupport.java:96)
at org.apache.activemq.transport.tcp.TcpTransport.run(TcpTransport.java:220)
at java.base/java.lang.Thread.run(Thread.java:840)
Caused by: java.io.EOFException
at java.base/java.io.DataInputStream.readInt(DataInputStream.java:386)
at org.apache.activemq.openwire.OpenWireFormat.unmarshal(OpenWireFormat.java:289)
at org.apache.activemq.transport.tcp.TcpTransport.readCommand(TcpTransport.java:241)
at org.apache.activemq.transport.tcp.TcpTransport.doRun(TcpTransport.java:233)
at org.apache.activemq.transport.tcp.TcpTransport.run(TcpTransport.java:216)
... 1 more
```
--
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: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] [AMQ-9820]: closed connections leaking into the pool when reconnectOnException is used [activemq]
Posted by "jeanouii (via GitHub)" <gi...@apache.org>.
jeanouii commented on PR #1564:
URL: https://github.com/apache/activemq/pull/1564#issuecomment-3665763335
@mattrpav Should be good
--
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: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] [AMQ-9820]: closed connections leaking into the pool when reconnectOnException is used [activemq]
Posted by "mattrpav (via GitHub)" <gi...@apache.org>.
mattrpav commented on code in PR #1564:
URL: https://github.com/apache/activemq/pull/1564#discussion_r2627396759
##########
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) {
Review Comment:
This looks like a good guard against the race condition
--
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: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] [AMQ-9820]: closed connections leaking into the pool when reconnectOnException is used [activemq]
Posted by "mattrpav (via GitHub)" <gi...@apache.org>.
mattrpav commented on code in PR #1564:
URL: https://github.com/apache/activemq/pull/1564#discussion_r2627394720
##########
activemq-jms-pool/src/test/java/org/apache/activemq/jms/pool/PooledConnectionSecurityExceptionTest.java:
##########
@@ -134,8 +134,12 @@ public void testFailureGetsNewConnectionOnRetry() throws Exception {
@Override
public boolean isSatisified() throws Exception {
- return connection1.getConnection() !=
- ((PooledConnection) pooledConnFact.createConnection("invalid", "credentials")).getConnection();
+ PooledConnection newConnection = (PooledConnection) pooledConnFact.createConnection("invalid", "credentials");
Review Comment:
How about try-with-resources here?
##########
activemq-jms-pool/src/test/java/org/apache/activemq/jms/pool/PooledConnectionSecurityExceptionTest.java:
##########
@@ -232,8 +236,12 @@ public void testFailoverWithInvalidCredentials() throws Exception {
@Override
public boolean isSatisified() throws Exception {
- return connection1.getConnection() !=
- ((PooledConnection) pooledConnFact.createConnection("invalid", "credentials")).getConnection();
+ PooledConnection newConnection = (PooledConnection) pooledConnFact.createConnection("invalid", "credentials");
Review Comment:
same.. t-w-r here?
--
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: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] [AMQ-9820]: closed connections leaking into the pool when reconnectOnException is used [activemq]
Posted by "mattrpav (via GitHub)" <gi...@apache.org>.
mattrpav merged PR #1564:
URL: https://github.com/apache/activemq/pull/1564
--
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: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact
Re: [PR] [AMQ-9820]: closed connections leaking into the pool when reconnectOnException is used [activemq]
Posted by "jeanouii (via GitHub)" <gi...@apache.org>.
jeanouii commented on code in PR #1564:
URL: https://github.com/apache/activemq/pull/1564#discussion_r2627416763
##########
activemq-jms-pool/src/test/java/org/apache/activemq/jms/pool/PooledConnectionSecurityExceptionTest.java:
##########
@@ -134,8 +134,12 @@ public void testFailureGetsNewConnectionOnRetry() throws Exception {
@Override
public boolean isSatisified() throws Exception {
- return connection1.getConnection() !=
- ((PooledConnection) pooledConnFact.createConnection("invalid", "credentials")).getConnection();
+ PooledConnection newConnection = (PooledConnection) pooledConnFact.createConnection("invalid", "credentials");
Review Comment:
Good point, it implements AutoClosable. I'll do that now
--
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: gitbox-unsubscribe@activemq.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscribe@activemq.apache.org
For additional commands, e-mail: gitbox-help@activemq.apache.org
For further information, visit: https://activemq.apache.org/contact