Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure the buffer is not released twice #3448

Merged
merged 2 commits into from
Sep 30, 2024
Merged

Ensure the buffer is not released twice #3448

merged 2 commits into from
Sep 30, 2024

Conversation

violetagg
Copy link
Member

When NettyOutbound#sendObject(java.lang.Object) is used, it is Netty's responsibility to release the buffer on success/error

Fixes #3406

When NettyOutbound#sendObject(java.lang.Object) is used,
it is Netty's responsibility to release the buffer on success/error

Fixes #3406
@violetagg violetagg added the type/bug A general bug label Sep 27, 2024
@violetagg violetagg added this to the 1.1.23 milestone Sep 27, 2024
Copy link
Member

@chemicL chemicL left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left just a small commenting request but otherwise it looks that this addresses the issue.

OutboundThen(NettyOutbound source, Publisher<Void> thenPublisher) {
this(source, thenPublisher, EMPTY_CLEANUP);
}

// This construction is used only with ChannelOperations#sendObject
// The implementation relies on Netty's promise that Channel#writeAndFlush will release the buffer on success/error
Copy link
Member

@chemicL chemicL Sep 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be worth to add another line comment that the release here happens only in case when we are sure that the processing doesn't delegate to Netty because of some failure before the exchange can be continued in the thenPublisher - which in a regular case takes ownership of the buffer.

@violetagg
Copy link
Member Author

@chemicL Thanks for the review!

@violetagg violetagg merged commit 172dd82 into 1.1.x Sep 30, 2024
14 checks passed
@violetagg violetagg deleted the issue-3406 branch September 30, 2024 19:10
violetagg added a commit that referenced this pull request Sep 30, 2024
violetagg added a commit that referenced this pull request Sep 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug A general bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Repeatedly releasing buf ,IllegalReferenceCountException: refCnt: 0, decrement: 1
2 participants