Repository navigation
AMQ-9392 - Rework fix for read check Timer leak - #1529
Merged
Merged
Conversation
…onnection fails (apache#1119) * Adding test. * Naive fix. * Fixing OpenWireConnectionTimeoutTest. - It seems checking configuredOk can't be done that early. - Falling back to let the Timer be created and make sure it is disposed of.
This commit updates the previous fix for preventing a read check Timer leak by the inactivity monitor when TCP connections fail This is a follow on to apache#1119 This updated version has the following improvements: 1) The logic to cancel timers and shutdown the executor has been consolidated into one location and only done during stopping of the transport. Previously, the update could stop the task/executor during normal broker operation (see next point) 2) stopConnectCheckTask() is called under normal circumstances during initialization of a connection and the previous change caused the read task to be cancelled and the executor to be stopped, only to be immediately restarted again during the first connection creation. 3) Null checks were added to guarantee no null pointer issues when referencing tasks/timers during stop 4) Helper methods and some comments were added to simplify the code and make it easier to understand what is going on.
jbonofre
approved these changes
Nov 5, 2025
jbonofre
left a comment
Member
There was a problem hiding this comment.
Much cleaner approach. Thanks !
Member
|
I'm just waiting Jenkins before merging (it worked locally on my machine). |
This test randomly creates 250 different types of connections to verify the read/write tasks are cancelled after all the connections have been cleaned up
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This commit updates the previous fix for preventing a read check Timer
leak by the inactivity monitor when TCP connections fail
This is a follow on to #1119
This updated version has the following improvements:
consolidated into one location and only done during stopping of the
transport. Previously, the update could stop the task/executor during
normal broker operation (see next point)
initialization of a connection and the previous change caused the read
task to be cancelled and the executor to be stopped, only to be
immediately restarted again during the first connection creation.
referencing tasks/timers during stop
and make it easier to understand what is going on.