Github user clebertsuconic commented on a diff in the pull request:
https://github.com/apache/activemq-artemis/pull/740#discussion_r76989578
--- Diff:
artemis-protocols/artemis-openwire-protocol/src/main/java/org/apache/activemq/artemis/core/protocol/openwire/OpenWireConnection.java
---
@@ -742,6 +756,25 @@ public void addConsumer(ConsumerInfo info) throws
Exception {
}
}
+ public void setConnectionEntry(ConnectionEntry connectionEntry) {
+ this.connectionEntry = connectionEntry;
+ }
+
+ public void setUpTtl(final long inactivityDuration, final long
inactivityDurationInitialDelay, final boolean useKeepAlive) {
+ this.useKeepAlive = useKeepAlive;
+ this.maxInactivityDuration = inactivityDuration;
+
+ protocolManager.getScheduledPool().schedule(new Runnable() {
--- End diff --
I see three things wrong here:
I - you are recreating the TTL mechanism already existing on
AbstractRemotingConnection. There's already a checker there for received pings
from the client.
Most of the issues I had on the heavy refactoring we did on artemis
openwire layer was around duplicated code. Having to figure out what was
abstract and what needed to be at the openwire level.
II - It seems you 'are not sending the packet to the client through the
Scheduler.
III - It seems this is leaking a future. I don't see you ever cancelling
the future upon disconnect. this will leak a connection through the scheduled
executor.
You must hold a Future from the call you made on schedule. And cancel after
the openWireConnection is closed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
