fix: close subscription consumers on GossClient.close() (leak from #33)#46
Merged
Merged
Conversation
…rce leak subscribe() created a DefaultClientConsumer but never stored it, so the JMS consumer and its listener thread were abandoned, never deregistered from the broker. Track subscription consumers and close each on client close() before the session closes. Each close() is individually guarded so one throwing consumer cannot strand the remaining consumers or the session/connection teardown. DefaultClientConsumer.close() guards a null messageConsumer (a partially-constructed consumer) and logs a close failure at error instead of printing the stack trace. Tests cover tracking, teardown, and the throwing consumer error path.
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
1 participant
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.
Problem
GossClient.subscribe()created aDefaultClientConsumerbut never stored it, so the JMS consumer and its listener thread were abandoned when the reference went out of scope, never deregistered from the broker. A long-running client that subscribes repeatedly leaks consumers.Fix
GossClient.close()before the session closes.close()is individually guarded so one throwing consumer cannot strand the remaining consumers or the session/connection teardown.DefaultClientConsumer.close()guards a nullmessageConsumer(a partially-constructed consumer whosecreateConsumer()failed and was swallowed in the constructor), and logs a close failure at error instead of printing the stack trace.Testing
GossClientConsumerLeakTest: subscribe tracks a consumer,close()closes and clears them, and a throwing consumer does not prevent the remaining teardown (the error-path regression guard)../gradlew :pnnl.goss.core:buildgreen.Reviews
Reviewed for code quality, security (no half-open authenticated session, removes a resource-exhaustion vector), and failure handling (the guarded loop closes a leak-on-error path).
Related
Recovers the substantive fix from the stale, unmergeable PR #33 (closed), reworked fresh on current develop. Tracking #40.