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 TcpDecoder memory leak #54

Merged
merged 2 commits into from
Aug 26, 2024
Merged

Conversation

JoeCqupt
Copy link
Contributor

@JoeCqupt JoeCqupt commented Aug 14, 2024

reproduce code

    @Test
    public void testTcpDecodeIllegalPacket1() {
        Codec codec = mock(Codec.class);
        doThrow(TRpcException.newFrameException(ErrorCode.TRPC_CLIENT_DECODE_ERR, "the request protocol is not trpc"))
                .when(codec).decode(any(), any());


        ProtocolConfig protocolConfig = new ProtocolConfig();
        // set batchDecoder true
        protocolConfig.setBatchDecoder(true);
        NettyCodecAdapter nettyCodecAdapter = NettyCodecAdapter.createTcpCodecAdapter(codec, protocolConfig);

        ChannelHandler decoder = nettyCodecAdapter.getDecoder();
        EmbeddedChannel embeddedChannel = new EmbeddedChannel();
        embeddedChannel.pipeline().addLast(decoder);

        ByteBuf byteBuf = AbstractByteBufAllocator.DEFAULT.heapBuffer();
        byteBuf.writeBytes("testTcpDecodeIllegalPacket1".getBytes(StandardCharsets.UTF_8));

        // write illegal packet
        EmbeddedChannel tmpEmbeddedChannel = embeddedChannel;
        DecoderException decoderException = Assert.assertThrows(DecoderException.class, () -> {
            tmpEmbeddedChannel.writeInbound(byteBuf);
        });

        Assert.assertTrue(decoderException.getCause() instanceof TRpcException);

        TRpcException tRpcException = (TRpcException) decoderException.getCause();
        Assert.assertEquals(tRpcException.getCode(), ErrorCode.TRPC_CLIENT_DECODE_ERR);

        Assert.assertEquals(byteBuf.refCnt(), 0);
    }

when TcpDecoder#decode() throw exception . ByteBuf param not released
expect refcnt 0 , actual 1

Solution
when decode excpetion . skip all readable bytes let ByteToMessageDecoder release ByteBuf

Copy link

github-actions bot commented Aug 14, 2024

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@JoeCqupt
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

I have read the CLA Document and I hereby sign the CLA

@JoeCqupt
Copy link
Contributor Author

recheck

Copy link

codecov bot commented Aug 14, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 69.34263%. Comparing base (a41c9aa) to head (9348276).
Report is 13 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@                  Coverage Diff                  @@
##                master         #54         +/-   ##
=====================================================
+ Coverage     69.27388%   69.34263%   +0.06874%     
- Complexity        4108        4111          +3     
=====================================================
  Files              428         428                 
  Lines            16774       16779          +5     
  Branches          1698        1700          +2     
=====================================================
+ Hits             11620       11635         +15     
+ Misses            4048        4039          -9     
+ Partials          1106        1105          -1     
Files Coverage Δ
...com/tencent/trpc/transport/netty/NettyChannel.java 59.37500% <100.00000%> (+2.70832%) ⬆️
...encent/trpc/transport/netty/NettyCodecAdapter.java 78.16092% <100.00000%> (+3.16092%) ⬆️

... and 7 files with indirect coverage changes

@JoeCqupt
Copy link
Contributor Author

@wardseptember PTAL

@JoeCqupt
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

@wardseptember
Copy link
Collaborator

感谢贡献,我们评估一下

@wardseptember
Copy link
Collaborator

I have read the CLA Document and I hereby sign the CLA

你需要用github账户提交push分支,然后才能授权成功

liuzengh added a commit to trpc-group/cla-database that referenced this pull request Aug 20, 2024
@wardseptember wardseptember merged commit acdbb07 into trpc-group:master Aug 26, 2024
16 checks passed
@wardseptember
Copy link
Collaborator

感谢贡献,已合入

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