Skip to content

Fix ByteBuf memory leak in TcpServerMock request decoders#47269

Merged
FabianMeiswinkel merged 4 commits into
users/fabianm/NettyBufferLeakFixesfrom
copilot/fix-memory-leak-in-tests
Nov 13, 2025
Merged

Fix ByteBuf memory leak in TcpServerMock request decoders#47269
FabianMeiswinkel merged 4 commits into
users/fabianm/NettyBufferLeakFixesfrom
copilot/fix-memory-leak-in-tests

Conversation

Copilot AI commented Nov 13, 2025

Copy link
Copy Markdown
Contributor

Description

Netty leak detection identified ByteBuf leaks in ServerRntbdRequestDecoder and ServerRntbdContextRequestDecoder test utilities. Both classes override channelRead() to conditionally bypass parent decode logic, but failed to release the ByteBuf reference when calling context.fireChannelRead() directly.

In Netty's reference counting model, when you bypass super.channelRead() and forward via context.fireChannelRead(), you must explicitly release your owned reference - the forwarding doesn't transfer ownership of your reference, it only passes a view to the next handler.

Changes:

  • Wrapped context.fireChannelRead() calls in try-finally blocks with ReferenceCountUtil.release()
  • Added import for io.netty.util.ReferenceCountUtil
  • Affects only test infrastructure code in azure-cosmos-tests

Leak trace before fix:

NETTY LEAK (traced) type=ByteBuf
ServerRntbdRequestDecoder.channelRead(ServerRntbdRequestDecoder.java:32)

All SDK Contribution checklist:

  • The pull request does not introduce [breaking changes]
  • CHANGELOG is updated for new features, bug fixes or other significant changes.
  • I have read the contribution guidelines.

General Guidelines and Best Practices

  • Title of the pull request is clear and informative.
  • There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, see this page.

Testing Guidelines

  • Pull request includes test coverage for the included changes.
Original prompt

This section details on the original issue you should resolve

<issue_title>[BUG]Memory leak in test code for ReactorNettyHttpClientTest due to bug in com.azure.cosmos.implementation.directconnectivity.TcpServerMock.rntbd.ServerRntbdRequestDecoder</issue_title>
<issue_description>Describe the bug
Test logs show the following netty buffer leak - likely caused in TcpMockServer. Pelase investigate and propose a fix.

2025-11-13T19:38:27.2272564Z 2025-11-13 19:38:26,901 [main] INFO CosmosNettyLeakDetectorFactory - LEAK DETECTION EVALUATION for test class com.azure.cosmos.implementation.http.ReactorNettyHttpClientTest
2025-11-13T19:38:28.2214264Z 2025-11-13 19:38:28,215 [main] ERROR CosmosNettyLeakDetectorFactory - NETTY LEAKS detected in test class: com.azure.cosmos.implementation.http.ReactorNettyHttpClientTest
2025-11-13T19:38:28.2215188Z
2025-11-13T19:38:28.2215514Z
2025-11-13T19:38:28.2215778Z NETTY LEAK (traced) type=ByteBuf records=
2025-11-13T19:38:28.2215994Z
2025-11-13T19:38:28.2216204Z Recent access records:
2025-11-13T19:38:28.2216529Z #1:
2025-11-13T19:38:28.2216964Z io.netty.buffer.AdvancedLeakAwareByteBuf.getUnsignedInt(AdvancedLeakAwareByteBuf.java:197)
2025-11-13T19:38:28.2217486Z io.netty.handler.codec.LengthFieldBasedFrameDecoder.getUnadjustedFrameLength(LengthFieldBasedFrameDecoder.java:468)
2025-11-13T19:38:28.2217942Z io.netty.handler.codec.LengthFieldBasedFrameDecoder.decode(LengthFieldBasedFrameDecoder.java:410)
2025-11-13T19:38:28.2218417Z io.netty.handler.codec.LengthFieldBasedFrameDecoder.decode(LengthFieldBasedFrameDecoder.java:333)
2025-11-13T19:38:28.2218922Z io.netty.handler.codec.ByteToMessageDecoder.decodeRemovalReentryProtection(ByteToMessageDecoder.java:530)
2025-11-13T19:38:28.2219393Z io.netty.handler.codec.ByteToMessageDecoder.callDecode(ByteToMessageDecoder.java:469)
2025-11-13T19:38:28.2219864Z io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:290)
2025-11-13T19:38:28.2220602Z io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:444)
2025-11-13T19:38:28.2221579Z io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
2025-11-13T19:38:28.2222359Z io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412)
2025-11-13T19:38:28.2222793Z io.netty.handler.ssl.SslHandler.unwrap(SslHandler.java:1519)
2025-11-13T19:38:28.2223267Z io.netty.handler.ssl.SslHandler.decodeJdkCompatible(SslHandler.java:1377)
2025-11-13T19:38:28.2224272Z io.netty.handler.ssl.SslHandler.decode(SslHandler.java:1428)
2025-11-13T19:38:28.2224789Z io.netty.handler.codec.ByteToMessageDecoder.decodeRemovalReentryProtection(ByteToMessageDecoder.java:530)
2025-11-13T19:38:28.2225235Z io.netty.handler.codec.ByteToMessageDecoder.callDecode(ByteToMessageDecoder.java:469)
2025-11-13T19:38:28.2225696Z io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:290)
2025-11-13T19:38:28.2226165Z io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:444)
2025-11-13T19:38:28.2226982Z io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
2025-11-13T19:38:28.2228154Z io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412)
2025-11-13T19:38:28.2228712Z io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1357)
2025-11-13T19:38:28.2229174Z io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:440)
2025-11-13T19:38:28.2229740Z io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
2025-11-13T19:38:28.2230216Z io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:868)
2025-11-13T19:38:28.2230715Z io.netty.channel.nio.AbstractNioByteChannel$NioByteUnsafe.read(AbstractNioByteChannel.java:166)
2025-11-13T19:38:28.2231155Z io.netty.channel.nio.NioEventLoop.processSelectedKey(NioEventLoop.java:796)
2025-11-13T19:38:28.2231589Z io.netty.channel.nio.NioEventLoop.processSelectedKeysOptimized(NioEventLoop.java:732)
2025-11-13T19:38:28.2232011Z io.netty.channel.nio.NioEventLoop.processSelectedKeys(NioEventLoop.java:658)
2025-11-13T19:38:28.2232392Z io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:562)
2025-11-13T19:38:28.2232852Z io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:998)
2025-11-13T19:38:28.2233293Z io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
2025-11-13T19:38:28.2233741Z io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
2025-11-13T19:38:28.2234143Z java.lang.Thread.run(Thread.java:750)
2025-11-13T19:38:28.2234510Z Azure/azure-sdk-for-java...


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

…questDecoder

When overriding channelRead() in ByteToMessageDecoder and bypassing the parent's
decode logic by calling context.fireChannelRead() directly, the ByteBuf reference
must be explicitly released to prevent memory leaks. Added ReferenceCountUtil.release()
in try-finally blocks to properly manage buffer lifecycle.

Co-authored-by: FabianMeiswinkel <19165014+FabianMeiswinkel@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix memory leak in ReactorNettyHttpClientTest Fix ByteBuf memory leak in TcpServerMock request decoders Nov 13, 2025
@FabianMeiswinkel FabianMeiswinkel marked this pull request as ready for review November 13, 2025 20:24
@FabianMeiswinkel FabianMeiswinkel merged commit ad522f1 into users/fabianm/NettyBufferLeakFixes Nov 13, 2025
2 checks passed
@FabianMeiswinkel FabianMeiswinkel deleted the copilot/fix-memory-leak-in-tests branch November 13, 2025 20:24
FabianMeiswinkel added a commit that referenced this pull request Nov 20, 2025
…ent instance detection during test execution (#47211)

* Test changes to add leak detection

* Update Configs.java

* Update CosmosNettyLeakDetectorFactory.java

* Update sdk/cosmos/azure-cosmos-encryption/src/test/java/com/azure/cosmos/encryption/CosmosNettyLeakDetectorFactory.java

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update CosmosNettyLeakDetectorFactory.java

* Fixes

* Fixes

* Update CosmosNettyLeakDetectorFactory.java

* Update RxDocumentClientImpl.java

* Fixes

* Update CosmosNettyLeakDetectorFactory.java

* Fixes

* Fixes

* Iterating on tests

* Fixing build warning

* Fixing memory leak

* Reverting production changes

* Iterating on test tools

* Cleaning-up dummy QueryFeedRangeState properly

* Update test-resources.json

* Update sdk/cosmos/azure-cosmos-tests/src/test/java/com/azure/cosmos/CosmosDiagnosticsE2ETest.java

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* [WIP] Fix Netty buffer and RxDocumentClientImpl leaks (#47213)

* Initial plan

* Improve JavaDoc phrasing in RxDocumentClientImpl

Co-authored-by: FabianMeiswinkel <19165014+FabianMeiswinkel@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: FabianMeiswinkel <19165014+FabianMeiswinkel@users.noreply.github.com>
Co-authored-by: Fabian Meiswinkel <fabianm@microsoft.com>

* NITs

* Move static block to class level in cosmos-encryption TestSuiteBase (#47216)

* Initial plan

* Move CosmosNettyLeakDetectorFactory.ingestIntoNetty() to class-level static block

Co-authored-by: FabianMeiswinkel <19165014+FabianMeiswinkel@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: FabianMeiswinkel <19165014+FabianMeiswinkel@users.noreply.github.com>

* Update SessionTest.java

* Update tests.yml

* Update CosmosNettyLeakDetectorFactory.java

* Test config

* Update CosmosNettyLeakDetectorFactory.java

* Updating TestNG

* Reverting TestNG to 7.9.0 (highest version still supporting Java8)

* Switching back to TestNG 7.5.1

* Enabling leak detection in unit tests

* Iterating on tests

* Update pom.xml

* Test changes (#47233)

* Update RntbdTransportClientTest.java

* Updating netty leak detection system properties

* Update CosmosNettyLeakDetectorFactory.java

* Test changes

* Prod memory leak fixes

* Test fixes

* Test fixes

* Users/fabianm/portfixes (#47252)

* Update WebExceptionRetryPolicy.java

* Update ThinClientStoreModel.java

* Test fixes

* Fix Netty ByteBuf leaks in StoreResponse and RetryContextOnDiagnosticTest (#47266)

* Initial plan

* Improve logging for ByteBufInputStream close failures

Change log level from debug to warn and catch Throwable instead of just IOException to make potential ByteBuf leak issues more visible.

Co-authored-by: FabianMeiswinkel <19165014+FabianMeiswinkel@users.noreply.github.com>

* Fix ByteBuf leak in RetryContextOnDiagnosticTest

Changed from Mono.just() to Mono.fromCallable() to defer StoreResponse creation, ensuring ByteBuf lifecycle is properly managed within each subscription rather than eagerly at mock setup time.

Co-authored-by: FabianMeiswinkel <19165014+FabianMeiswinkel@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: FabianMeiswinkel <19165014+FabianMeiswinkel@users.noreply.github.com>

* Update EncryptionAsyncApiCrudTest.java

* Fix ByteBuf memory leak in TcpServerMock request decoders (#47269)

* Initial plan

* Fix memory leak in ServerRntbdRequestDecoder and ServerRntbdContextRequestDecoder

When overriding channelRead() in ByteToMessageDecoder and bypassing the parent's
decode logic by calling context.fireChannelRead() directly, the ByteBuf reference
must be explicitly released to prevent memory leaks. Added ReferenceCountUtil.release()
in try-finally blocks to properly manage buffer lifecycle.

Co-authored-by: FabianMeiswinkel <19165014+FabianMeiswinkel@users.noreply.github.com>

* Replace ReferenceCountUtil.release with safeRelease

* Replace ReferenceCountUtil.release with safeRelease

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: FabianMeiswinkel <19165014+FabianMeiswinkel@users.noreply.github.com>
Co-authored-by: Fabian Meiswinkel <fabianm@microsoft.com>

* Update ThinClientStoreModel.java

* Fixing test issues

* Disable netty leak detection in RetrycontextOnDiagnosticTest

* Test and diagnostics improvements

* Test fixes and more breadcrumbs

* Test fixes

* Test fixes

* Test fixes

* Update OrderbyDocumentQueryTest.java

* Test fixes

* Reverting too noisy logs

* Test fixes

* Fixing POMs

* React to code review feedback

* Update pom.xml

* Addresses code review feedback

* Update sdk/cosmos/azure-cosmos/src/main/java/com/azure/cosmos/implementation/directconnectivity/rntbd/RntbdRequestDecoder.java

Co-authored-by: Annie Liang <64233642+xinlian12@users.noreply.github.com>

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Co-authored-by: FabianMeiswinkel <19165014+FabianMeiswinkel@users.noreply.github.com>
Co-authored-by: Annie Liang <64233642+xinlian12@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants