Skip to content

Syslog disable autorelease (7.0)#25141

Open
fpetersen-gl wants to merge 1 commit into7.0from
backport-7.0/syslog_disable_autorelease
Open

Syslog disable autorelease (7.0)#25141
fpetersen-gl wants to merge 1 commit into7.0from
backport-7.0/syslog_disable_autorelease

Conversation

@fpetersen-gl
Copy link
Contributor

Note: This is a backport of ##25120 to 7.0.

Resolves #24336

This PR adds some unit tests to #24525 and proposes a simpler memory management approach to handling issues due to incomplete Syslog frames.

Details

SimpleChannelInboundHandler(boolean autoRelease) — The constructor accepts a boolean to control auto-release behavior. When false, the handler does NOT release the message after channelRead0() returns. This transfers buffer ownership entirely to whoever channelRead0() passes it to. Since ByteToMessageDecoder.channelRead() already manages buffer lifecycle correctly (its cumulator absorbs or copies-and-releases the input), there's no need for external tracking at all.

When a Netty handler needs to pass a buffer to a downstream ByteToMessageDecoder that isn't in the pipeline, the cleanest pattern is to disable SimpleChannelInboundHandler's auto-release via super(false) rather than fighting it with retain()/tracking. The downstream decoder already manages buffer lifecycle through its cumulator. The only responsibility left is forwarding channelInactive so the decoder releases its internal cumulation on connection close.

* Keeping track of retained buffers, always releasing them after processing

* Removed logging, added cl

* Removed system.out.println from test

* Try to keep track of as few ByteBufs as possible by removing the reference to already freed ones while reading from an open channel.

* alternate approach to buffer release

* unit tests for edge cases

* CL

---------

Co-authored-by: Florian Petersen <188503754+fpetersen-gl@users.noreply.github.com>

(cherry picked from commit 41f67ba)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants