NettyAdaptiveCumulator incorrectly releases ByteBuf in twice

[hengyunabc]

What version of gRPC-Java are you using?

Version 1.51.0 introduced this issue.

What is your environment?

Any

What did you expect to see?

What did you see instead?

Steps to reproduce the bug

io.netty.handler.codec.DecoderException: io.netty.util.IllegalReferenceCountException: refCnt: 0, decrement: 1
	at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:294)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:444)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
	at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:412)
	at io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1410)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:440)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:420)
	at io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:919)
	at io.netty.channel.local.LocalChannel.readInbound(LocalChannel.java:299)
	at io.netty.channel.local.LocalChannel.doBeginRead(LocalChannel.java:322)
	at io.netty.channel.AbstractChannel$AbstractUnsafe.beginRead(AbstractChannel.java:834)
	at io.netty.channel.DefaultChannelPipeline$HeadContext.read(DefaultChannelPipeline.java:1362)
	at io.netty.channel.AbstractChannelHandlerContext.invokeRead(AbstractChannelHandlerContext.java:835)
	at io.netty.channel.AbstractChannelHandlerContext.read(AbstractChannelHandlerContext.java:814)
	at io.netty.handler.codec.http2.Http2ConnectionHandler.read(Http2ConnectionHandler.java:539)
	at io.netty.channel.AbstractChannelHandlerContext.invokeRead(AbstractChannelHandlerContext.java:839)
	at io.netty.channel.AbstractChannelHandlerContext.read(AbstractChannelHandlerContext.java:814)
	at io.netty.channel.DefaultChannelPipeline.read(DefaultChannelPipeline.java:1004)
	at io.netty.channel.AbstractChannel.read(AbstractChannel.java:290)
	at io.netty.channel.DefaultChannelPipeline$HeadContext.readIfIsAutoRead(DefaultChannelPipeline.java:1422)
	at io.netty.channel.DefaultChannelPipeline$HeadContext.channelReadComplete(DefaultChannelPipeline.java:1417)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelReadComplete(AbstractChannelHandlerContext.java:482)
	at io.netty.channel.AbstractChannelHandlerContext.invokeChannelReadComplete(AbstractChannelHandlerContext.java:463)
	at io.netty.channel.DefaultChannelPipeline.fireChannelReadComplete(DefaultChannelPipeline.java:925)
	at io.netty.channel.local.LocalChannel.readInbound(LocalChannel.java:302)
	at io.netty.channel.local.LocalChannel.finishPeerRead0(LocalChannel.java:445)
	at io.netty.channel.local.LocalChannel.access$400(LocalChannel.java:50)
	at io.netty.channel.local.LocalChannel$5.run(LocalChannel.java:403)
	at io.netty.util.concurrent.AbstractEventExecutor.runTask(AbstractEventExecutor.java:174)
	at io.netty.channel.DefaultEventLoop.run(DefaultEventLoop.java:54)
	at io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:997)
	at io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
	at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
	at java.lang.Thread.run(Thread.java:748)
Caused by: io.netty.util.IllegalReferenceCountException: refCnt: 0, decrement: 1
	at io.netty.util.internal.ReferenceCountUpdater.toLiveRealRefCnt(ReferenceCountUpdater.java:83)
	at io.netty.util.internal.ReferenceCountUpdater.release(ReferenceCountUpdater.java:147)
	at io.netty.buffer.AbstractReferenceCountedByteBuf.release(AbstractReferenceCountedByteBuf.java:101)
	at io.grpc.netty.NettyAdaptiveCumulator.cumulate(NettyAdaptiveCumulator.java:95)
	at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:288)
	... 33 common frames omitted

In the case below, NettyAdaptiveCumulator incorrectly releases ByteBuf in twice.

• io.grpc.netty.NettyAdaptiveCumulator.cumulate(ByteBufAllocator alloc, ByteBuf cumulation, ByteBuf in)

• cumulation: UnpooledHeapByteBuf(ridx: 9, widx: 14, cap: 14/14)

• in: UnpooledHeapByteBuf(ridx: 0, widx: 8, cap: 8/8)

---

• The finally block of NettyAdaptiveCumulator#mergeWithCompositeTail is released once
• Release again in the finally block of NettyAdaptiveCumulator#cumulate

[sanjaypujare]

@sergiitk may be you can take a look?

[larry-safran]

It looks to me that there is one (and only one) way a double release could happen. It is if an exception was thrown in line 211 of NettyAdaptiveCumulator.java as mergeWithCompositeTail only has that line of logic after in.release() on line 207.
This is because after the call to addInput calls mergeWithCompositeTail it immediately sets in = null and the release of in is gated by in != null. However, composite.readerIndex calls AbstractBuffer.readerIndex which can do a checkIndexBounds (default behavior) which can throw an IndexOutOfBoundsException.

It would be good to change

      if (in != null) {
        in.release();
      }

to

       if (in != null && in.refCnt() > 0) {
        in.release();
      }