fix TcpDecoder memory leak by JoeCqupt · Pull Request #54 · trpc-group/trpc-java · GitHub
Skip to content

fix TcpDecoder memory leak#54

Merged
wardseptember merged 2 commits into
trpc-group:masterfrom
JoeCqupt:master
Aug 26, 2024
Merged

fix TcpDecoder memory leak#54
wardseptember merged 2 commits into
trpc-group:masterfrom
JoeCqupt:master

Conversation

@JoeCqupt

@JoeCqupt JoeCqupt commented Aug 14, 2024

Copy link
Copy Markdown
Contributor

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

@github-actions

github-actions Bot commented Aug 14, 2024

Copy link
Copy Markdown

@JoeCqupt

Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

recheck

@codecov

codecov Bot commented Aug 14, 2024

Copy link
Copy Markdown

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
Copy Markdown
Contributor Author

@wardseptember PTAL

@JoeCqupt

Copy link
Copy Markdown
Contributor Author

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

@wardseptember

Copy link
Copy Markdown
Collaborator

感谢贡献,我们评估一下

@wardseptember

Copy link
Copy Markdown
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
@wardseptember

Copy link
Copy Markdown
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