[Flaky Test] Throw an io.netty.util.IllegalReferenceCountException

[jerqi]

Code of Conduct

• I agree to follow this project's Code of Conduct

Search before asking

• I have searched in the issues and found no similar issues.

Describe the flaky test

Caused by: io.netty.util.IllegalReferenceCountException: refCnt: 0
at io.netty.buffer.AbstractByteBuf.ensureAccessible(AbstractByteBuf.java:1454)
at io.netty.buffer.AbstractByteBuf.checkIndex(AbstractByteBuf.java:1383)
at io.netty.buffer.PooledByteBuf.duplicateInternalNioBuffer(PooledByteBuf.java:196)
at io.netty.buffer.PooledByteBuf.nioBuffer(PooledByteBuf.java:213)
at io.netty.buffer.AbstractByteBuf.nioBuffer(AbstractByteBuf.java:1231)
at org.apache.uniffle.common.netty.buffer.NettyManagedBuffer.nioByteBuffer(NettyManagedBuffer.java:48)
at org.apache.uniffle.common.ShuffleIndexResult.getIndexData(ShuffleIndexResult.java:84)

Actions URL

https://github.com/apache/incubator-uniffle/actions/runs/12158115239/attempts/1

Are you willing to submit PR?

• Yes I am willing to submit a PR!

[jerqi]

Maybe we shouldn't use PooledByteBuf. Because we will release the memory after we used the memory. You can see

incubator-uniffle/storage/src/main/java/org/apache/uniffle/storage/handler/impl/DataSkippableReadHandler.java

Line 82 in 28475fc

shuffleIndexResult.release();

cc @maobaolong @advancedxy @rickyma @LuciferYang WDYT?

[rickyma]

Users can choose whatever kind of bytebuf they want:
client: #1767
server: #2145

We basically will release every bytebuf, but it does not mean that we don't need PooledByteBuf.
We use PooledByteBuf to reduce the number of interactions with the operating system for memory allocation and deallocation. This helps to avoid potential issues such as abnormal growth of RSS (Resident Set Size) memory, which might occur due to frequent and inefficient memory management (especially when using glibc, that's why we recommend using mimalloc).

[jerqi]

Users can choose whatever kind of bytebuf they want: client: #1767 server: #2145

We basically will release every bytebuf, but it does not mean that we don't need PooledByteBuf. We use PooledByteBuf to reduce the number of interactions with the operating system for memory allocation and deallocation. This helps to avoid potential issues such as abnormal growth of RSS (Resident Set Size) memory, which might occur due to frequent and inefficient memory management (especially when using glibc, that's why we recommend using mimalloc).

But it will throw a ref count exception in our UT. I suspect that we released a reused bytebuf in the ByteBuf pool.

[rickyma]

I didn't look into the code carefully. I suppose this should be a bug. @maobaolong

Similar to #1628?

[jerqi]

I didn't look into the code carefully. I suppose this should be a bug. @maobaolong

Similar to #1628?

Yes, they are similar.

[jerqi]

I suspect that the issue is caused by TransportFrameDecoder

  @Override
  public void handlerRemoved(ChannelHandlerContext ctx) throws Exception {
    // Release all buffers that are still in our ownership.
    // Doing this in handlerRemoved(...) guarantees that this will happen in all cases:
    //     - When the Channel becomes inactive
    //     - When the decoder is removed from the ChannelPipeline
    for (ByteBuf b : buffers) {
      b.release();
    }
    buffers.clear();
    headerBuf.release();
    super.handlerRemoved(ctx);
  }