Fix memory leak in RequestResponseChannel by draining SendLinkHandler link credits (#47261)#49772
Open
ksalazar-91 wants to merge 2 commits into
Open
Conversation
… link credits (Azure#47261) RequestResponseChannel never subscribed to SendLinkHandler.getLinkCredits(), a unicast unbounded-buffer sink fed on every AMQP flow frame, so credits buffered indefinitely for long-lived cached management channels. The channel now drains the credit flux (disposed on close), mirroring ReactorSender. Adds a regression test and CHANGELOG entry. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: f2d0e805-73ed-4d31-9bb2-b4541c325600
Contributor
|
Thank you for your contribution @ksalazar-91! We will review the pull request and get back to you soon. |
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request fixes an unbounded memory leak in azure-core-amqp where RequestResponseChannel failed to subscribe to (and therefore drain) SendLinkHandler’s link-credit flux, causing credits to buffer indefinitely for long-lived cached channels.
Changes:
- Subscribe to
sendLinkHandler.getLinkCredits()inRequestResponseChanneland register the resultingDisposablefor proper cleanup on channel termination. - Add a regression test asserting the credit flux is subscribed to and that the subscription is disposed on channel close.
- Document the fix in the
azure-core-amqpchangelog under2.13.0-beta.1.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/implementation/RequestResponseChannel.java | Drains send-link credit emissions by subscribing and disposing via the channel’s subscriptions composite. |
| sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/implementation/RequestResponseChannelTest.java | Adds a regression test ensuring the credit flux is subscribed and cancelled on close. |
| sdk/core/azure-core-amqp/CHANGELOG.md | Adds a Bugs Fixed entry describing the leak and the mitigation. |
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.
Summary
Fixes a memory leak in
RequestResponseChannel(azure-core-amqp), reported in #47261.The channel's
SendLinkHandlerexposes link credits viagetLinkCredits(), a unicast, unbounded-buffer sink (Sinks.many().unicast().onBackpressureBuffer()) that emits a value on every AMQP flow frame (onLinkFlow).ReactorSendersubscribes to and drains this flux, butRequestResponseChannel— which backs all management / request-response operations (lock renewal, peek, schedule, session state, etc.) — only subscribed to the send link's endpoint states and never to its credits. With no subscriber, every emitted credit is retained in the sink's buffer for the lifetime of the channel.Because the request-response channel is cached (
RequestResponseChannelCache) and lives for the whole connection, a long-running, high-throughput client accumulates one bufferedIntegerper flow frame indefinitely, matching the reporter's heap analysis:Fix
RequestResponseChannelnow subscribes to (and thereby drains)sendLinkHandler.getLinkCredits(), registering theDisposablein itssubscriptionscomposite so it is disposed on channel close. This mirrors the channel's existing endpoint-state / delivered-message subscriptions and howReactorSenderalready drains credits.RequestResponseChannelcorrelates responses viaunconfirmedSendsand does not use AMQP link-credit flow control, so discarding the credit values is safe and behavior-preserving.Reason
Steady, unbounded heap growth for long-lived Service Bus / AMQP clients under sustained load, eventually leading to
OutOfMemoryError.Fixes #47261.
Verification
RequestResponseChannelTest#subscribesToSendLinkCreditsToPreventLeakasserts the channel subscribes to the credit flux and disposes the subscription on close.RequestResponseChannelTest(16/16) andSendLinkHandlerTest(7/7) pass; checkstyle and spotbugs are clean.SendLinkHandlerover 2,000,000 simulated flow frames: without the drain ~38 MB retained (all 2M credits replayed to the first subscriber); with the drain ~0 MB retained.Notes for reviewers
azure-core-amqp;SendLinkHandleris unchanged, so senders that legitimately consume credits are unaffected.2.13.0-beta.1→ Bugs Fixed.