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 2026/02/06 09:19:38 UTC
[PR] [#TODO] connection and pool improvements [activemq]
jeanouii opened a new pull request, #1657:
URL: https://github.com/apache/activemq/pull/1657
(no comment)
--
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] [#TODO] connection and pool improvements [activemq]
Posted by "mattrpav (via GitHub)" <gi...@apache.org>.
mattrpav commented on PR #1657:
URL: https://github.com/apache/activemq/pull/1657#issuecomment-3874343916
@cshannon thoughts on adding AutoCloseable to PooledConnectionFactory in a patch release? I think this will make IDEs bark at users' existing code and create false negatives.
Most things today (Camel routes, etc) use start()/stop() and not a try-with-resources.
--
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] [#TODO] connection and pool improvements [activemq]
Posted by "jeanouii (via GitHub)" <gi...@apache.org>.
jeanouii commented on PR #1657:
URL: https://github.com/apache/activemq/pull/1657#issuecomment-3860810775
When using the resource adapter inside an app server for example (Apache TomEE), the connection pool might be created and destroyed multiple times without restart the entire server (deploy / undeploy). It delegates to stop(). For tests it's convenient to avoid leaking a connection factory from one test to the other or having to cope with try/catch.
It's not the end of the world. I can revert that and rework all the tests. I don't think it harms though.
--
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] [#TODO] connection and pool improvements [activemq]
Posted by "jbonofre (via GitHub)" <gi...@apache.org>.
jbonofre commented on PR #1657:
URL: https://github.com/apache/activemq/pull/1657#issuecomment-3878269660
I think we could consider `AutoCloseable` as a kind of backward compatible: the user can use try on resource or the "old style" 😄
I'm not fan of having a subclass because there's a risk it's not actually used/visible.
--
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] [#TODO] connection and pool improvements [activemq]
Posted by "cshannon (via GitHub)" <gi...@apache.org>.
cshannon commented on PR #1657:
URL: https://github.com/apache/activemq/pull/1657#issuecomment-3874645109
> @cshannon thoughts on adding AutoCloseable to PooledConnectionFactory in a patch release? I think this will make IDEs bark at users' existing code and create false negatives.
I guess it could cause some warnings to be thrown etc. in an IDE but it shouldn't break anyone's code or have any real other effects so I don't really think it's a big deal to add it. It also wouldn't hurt to just add it in 6.3.0 (ie not backport to a patch release). I guess it depends on how useful it is to add it for fixing tests and backports.
I don't really think we need a subclass because adding the interface on the existing class won't actually cause any issues.
--
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] [#TODO] connection and pool improvements [activemq]
Posted by "jeanouii (via GitHub)" <gi...@apache.org>.
jeanouii commented on PR #1657:
URL: https://github.com/apache/activemq/pull/1657#issuecomment-3872565850
ok to merge @mattrpav
--
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] [#TODO] connection and pool improvements [activemq]
Posted by "cshannon (via GitHub)" <gi...@apache.org>.
cshannon commented on PR #1657:
URL: https://github.com/apache/activemq/pull/1657#issuecomment-3878761324
> The standard/default use case for PooledConnectionFactory is to _not_ use a try-with-resources. Feels like adding this is for the _test_ use case vs the primary coded use case.
Thinking about it more that makes sense as it's a pooled factory so users really wouldn't ever be closing it immediately with try-with-resources. I can't really think of a use case where you'd use a pooled factory and immediately close it with try-with-resources as if you were going to do that you'd just use a normal connection.
For the analysis part I was mostly thinking of it from an IDE issue (which isn't that big of a deal but a little annoying) but the static analysis issue of builds breaking with things like spot bugs is certainly a problem and would produce annoying false negatives.
I guess based on that we should leave it off and go back to try/finally. I'm not really in favor of a subclass unless there is a real use case, or another option is we just add a subclass only in the testing package for unit tests because it doesn't seem beneficial to add autocloseable if there is no use case for a real application.
--
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] [#TODO] connection and pool improvements [activemq]
Posted by "mattrpav (via GitHub)" <gi...@apache.org>.
mattrpav commented on PR #1657:
URL: https://github.com/apache/activemq/pull/1657#issuecomment-3860507344
I need to think through impacts of making PooledConnectionFactory AutoCloseable. It usually is more of an init/destroy start/stop lifecycle since it is long lived for the lifetime of the JVM.
Adding AutoCloseable may lead to a lot of users getting WARN from IDE
--
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] [#TODO] connection and pool improvements [activemq]
Posted by "jeanouii (via GitHub)" <gi...@apache.org>.
jeanouii commented on PR #1657:
URL: https://github.com/apache/activemq/pull/1657#issuecomment-3876978804
It's backward compatible and won't break for sure.
On integration code (not only on tests), like in TomEE for example, having the try-with-resources is great for deploy/undeploy.
It was convenient for me to cleanup the tests, if that's something you guys are annoyed with, I'm happy to remove it and replace try-with-resources with try/catch/finally in tests.
Let me know what you prefer.
--
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] [#TODO] connection and pool improvements [activemq]
Posted by "mattrpav (via GitHub)" <gi...@apache.org>.
mattrpav commented on PR #1657:
URL: https://github.com/apache/activemq/pull/1657#issuecomment-3878481620
Hear my out on this point though-- AutoCloseable isn't just another interface tagged to a class. Static code scanners and bug finders look for code patterns to and generate reports that drive approval for pull requests. While not as impactful as 'Serializable' it is in this category of more 'special' interfaces.
The standard/default use case for PooledConnectionFactory is to _not_ use a try-with-resources. Feels like adding this is for the _test_ use case primarily vs the primary coded use case.
--
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] [#TODO] connection and pool improvements [activemq]
Posted by "cshannon (via GitHub)" <gi...@apache.org>.
cshannon commented on PR #1657:
URL: https://github.com/apache/activemq/pull/1657#issuecomment-3878020281
> Let me know what you prefer.
I would prefer to keep it as it is backwards compatible and provides a lot of benefits and there is no risk of breaking anything. Adding another subclass doesn't make sense to me as someone would now have to know to use 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: 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] [#TODO] connection and pool improvements [activemq]
Posted by "jeanouii (via GitHub)" <gi...@apache.org>.
jeanouii commented on PR #1657:
URL: https://github.com/apache/activemq/pull/1657#issuecomment-3880286220
I removed the AutoClosable if that's the consensus.
--
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-9860] [AMQ-9861] connection async exception handling and activemq pool tests connection leaks to other tests [activemq]
Posted by "jeanouii (via GitHub)" <gi...@apache.org>.
jeanouii merged PR #1657:
URL: https://github.com/apache/activemq/pull/1657
--
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