Close the producer sendTopicMessage creates - #1850
Merged
Merged
Conversation
sendQueueMessage has always closed its MessageProducer in a finally; sendTopicMessage never did, so every publish left one on the session. That matters because the sessions doing the publishing are long-lived: SimDataServer publishes every data and export event to ClientStatusTopic on a session created at startup, and SimulationStateMachine and StatusMessage do the same. The producers accumulated for the life of the server -- one per status update, including every export progress event. Counting producers opened and closed in-process, five publishes leaked five producers before this change. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SY1XHgTZXZPqUECo2VBAWg
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.
Last of the JMS resource problems found while working through the export-progress incident
(#1837, #1838, #1839, #1842, #1844, #1845, and the consumer-close PR this follows).
The leak
sendQueueMessagehas always closed itsMessageProducerin afinally.sendTopicMessagenever did:
Every publish left a producer on the session.
Why it matters
The sessions doing the publishing are long-lived, so nothing ever cleaned them up:
SimDataServer— data events, export eventsClientStatusTopiccreateProducerSession()at startupSimulationStateMachineServiceControlTopicStatusMessage.sendToClientClientStatusTopicOne leaked producer per status update, for the life of the server — including every export
progress event, which is what made the original incident's message storm expensive in the
first place.
Verification
sendTopicMessageDoesNotLeakProducerspublishes five messages and requires the producercount to return to zero. Control, with the production file reverted and the test kept:
Five publishes, five leaked producers.
The count is taken in-process, by wrapping the JMS objects in proxies that tally
createProduceragainstclose, rather than querying the broker or turning on JMX. That isdeliberate: broker-side producer registration is asynchronous, and a count that depends on it
would be exactly the kind of racy assertion that passed locally and failed in CI earlier in
this series. This one is exact.
vcell-serverFast group: 62 passed.🤖 Generated with Claude Code
https://claude.ai/code/session_01SY1XHgTZXZPqUECo2VBAWg