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

fix: Fix ByteBuf leak in remoting transport. #1636

Closed
wants to merge 1 commit into from

Conversation

He-Pin
Copy link
Member

@He-Pin He-Pin commented Dec 28, 2024

Motivation:
refs: #1634

Modification:
Call byteBuf.release after bytes read.

Result:
No leaks.

Sorry for everyone.

It's using retainedSlice in LengthFieldBasedFrameDecoder

protected ByteBuf extractFrame(ChannelHandlerContext ctx, ByteBuf buffer, int index, int length) {
    return buffer.retainedSlice(index, length);
}

@He-Pin He-Pin changed the base branch from main to 1.1.x December 28, 2024 14:36
@He-Pin He-Pin added bug Something isn't working backport labels Dec 28, 2024
@He-Pin He-Pin added this to the 1.1.3 milestone Dec 28, 2024
@pjfanning pjfanning changed the base branch from 1.1.x to main December 28, 2024 14:42
@pjfanning
Copy link
Contributor

changed back to main branch - the PR was messed up when it was set to merge to 1.1.x branch and would have brought lots of unrelated changes

@pjfanning pjfanning closed this Dec 28, 2024
@He-Pin He-Pin deleted the 1.1.x-leakFix branch December 28, 2024 14:48
@pjfanning
Copy link
Contributor

pjfanning commented Dec 28, 2024

This should not be attempted while #1635 is in progress. We need 1 PR to review, not 2. We can get 1 PR right and look into backporting when the first PR is right.

@He-Pin He-Pin removed bug Something isn't working backport labels Dec 28, 2024
@He-Pin He-Pin removed this from the 1.1.3 milestone Dec 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants