[Solace] Close the HTTP response content stream in BrokerResponse#39404
[Solace] Close the HTTP response content stream in BrokerResponse#39404PDGGK wants to merge 1 commit into
Conversation
BrokerResponse read the SEMP HTTP response body via a BufferedReader but never closed the underlying InputStream, so every SEMP call leaked the HTTP response content stream (the HttpResponse is never disconnected elsewhere either). Wrap the reader in try-with-resources so the stream is always closed once the body has been read, and add a regression test.
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a resource leak in the BrokerResponse class where the HTTP response input stream was not being closed after being read. By wrapping the stream processing in a try-with-resources block, the fix ensures that connections are released properly, improving the stability and resource efficiency of SEMP calls. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
|
assign set of reviewers |
There was a problem hiding this comment.
Code Review
This pull request updates BrokerResponse to use a try-with-resources block when reading the response content, ensuring that the underlying InputStream is closed and preventing potential HTTP connection leaks. It also introduces a new test class BrokerResponseTest to verify that the stream is properly closed and that null content is handled correctly. There are no review comments to address, and I have no further feedback to provide.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
Assigning reviewers: R: @Abacn for label java. Note: If you would like to opt out of this review, comment Available commands:
The PR bot will only process comments in the main thread (not review comments). |
Fixes #39403
BrokerResponseread the SEMP HTTP response body through aBufferedReaderbut never closed the underlyingInputStream. SinceBrokerResponse.fromHttpResponseis fedHttpResponse#getContent()andSempBasicAuthClientExecutor(getQueueResponse/createQueueResponse/createSubscriptionResponse) never disconnects the response or closes the stream elsewhere, every SEMP call leaked the HTTP response content stream.This wraps the reader in a try-with-resources so the stream is always closed once the body has been read.
Stream#lines()already surfaces read errors asUncheckedIOException, so the constructor keeps the same unchecked failure contract; a close failure is wrapped the same way.Added
BrokerResponseTestverifying that the content is still parsed correctly and that the input stream is closed after construction.