[
https://issues.apache.org/jira/browse/CASSANDRA-21766?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18130610#comment-18130610
]
Francisco Guerrero commented on CASSANDRA-21766:
------------------------------------------------
Noting approval from [~jchovatia] in the PRs
> PreV5Handlers.toFlushItem may leak memory on disconnected channel
> -----------------------------------------------------------------
>
> Key: CASSANDRA-21766
> URL: https://issues.apache.org/jira/browse/CASSANDRA-21766
> Project: Apache Cassandra
> Issue Type: Bug
> Components: Legacy/Streaming and Messaging
> Reporter: Yuqi Yan
> Assignee: Yuqi Yan
> Priority: Normal
> Fix For: 4.1.x, 5.0.x, 6.0.x, 7.x
>
> Attachments: CASSANDRA-21766-cassandra-4.1-ci_summary.html,
> CASSANDRA-21766-cassandra-5.0-ci_summary.html,
> CASSANDRA-21766-cassandra-6.0-ci_summary.html,
> CASSANDRA-21766-trunk-ci_summary.html, image-2026-10-16-15-58-48-345.png
>
> Time Spent: 40m
> Remaining Estimate: 0h
>
> We patch the recent fix CASSANDRA-21508 to our Cassandra 4.1 branch and
> seeing a rise in off heap memory usage that correlates with the deployment:
> !image-2026-10-16-15-58-48-345.png|width=390,height=180!
> We spent some time to investigate that and it seems that CASSANDRA-21508
> introduced a Netty memory leaking scenario when the connection is destroyed
> when there is still query in flight. This seems only affecting PreV5
> protocols.
> * CASSANDRA-21508 moved response encoding for native protocol V4 and earlier
> off the Netty pipeline (PreV5Handlers.ProtocolEncoder) and onto the request
> worker.
> ** PreV5Handlers.LegacyDispatchHandler.toFlushItem now calls
> {*}response.encode(...){*}, which allocates the response body as a pooled
> direct ByteBuf (CBUtil.allocator) before the item reaches the Flusher.
> * Flusher.processUnframedResponse then calls channel.write(responseEnvelope,
> channel.voidPromise()). Envelope does not implement ReferenceCounted.
> ** Releasing its body depends on Envelope.Encoder in the pipeline turning it
> into a ByteBuf that Netty owns.
> * Suppose the client disconnects while a request is in flight. By the time
> the Flusher task runs on the event loop, the channel can be closed and
> deregistered. On channelUnregistered, DefaultChannelPipeline removes all
> handlers (HeadContext.channelUnregistered → destroy()).
> * The write then skips Envelope.Encoder and goes straight to the head
> context and AbstractUnsafe.write. There outboundBuffer is null, so Netty
> calls ReferenceCountUtil.release(msg), which does nothing for a
> non-ReferenceCounted Envelope. Because the write uses the void promise, the
> failure is silent.
> * LegacyDispatchHandler.releaseItem releases only the request envelope. Its
> comment still says the pipeline releases the response, which stopped being
> true with CASSANDRA-21508. Nothing else holds the response body, so it leaks.
> Note : the above summary is generated with help from AI, but I myself read
> through the code path carefully.
> In my own word, pre 21508, the byte buffer was allocated and freed inside the
> event loop task, and since destroying the connection and the pipelines also
> happen in the event loop, the buffer lifecycle is handled correctly. Post
> 21508, the buffer is allocated outside of event loop and there is no check on
> the channel status before attempting to write into the
> already-destroyed-pipeline, causing the leak.
>
> From the existing comment it says
> {code:java}
> private void releaseItem(Flusher.FlushItem<Message.Response> item)
> {
> // Note: in contrast to the equivalent for V5 protocol,
> CQLMessageHandler::release(FlushItem item),
> // this does not release the FlushItem's Message.Response. In V4, the
> buffers for the response's body
> // and serialised header are emitted directly down the Netty pipeline
> from Envelope.Encoder, so
> // releasing them is handled by the pipeline itself.
> long itemSize = item.request.header.bodySizeInBytes;
> item.request.release(); {code}
> But when the pipeline is destroyed, no one will be responsible to free the
> buffers for the response's body allocated outside of the event loop
> I have a simple unit test that can reproduce this leak, will attach in link
> soon.
> As for the fix, a naive thinking will be checking the channel status before
> trying to flush any item there, i.e.
> {code:java}
> private void processUnframedResponse(FlushItem.Unframed flush)
> {
> if (!flush.channel.isOpen())
> {
> flush.responseEnvelope.release();
> return;
> }
> flush.channel.write(flush.responseEnvelope, flush.channel.voidPromise());
> channels.add(flush.channel);
> }{code}
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
