From 87392634d2025f9a236931aa64683b9169ce4375 Mon Sep 17 00:00:00 2001
From: Zxy <1060676865@qq.com>
Date: Fri, 5 Feb 2021 21:53:58 +0800
Subject: [PATCH] Fix memory release failure when "maxNumElems == 1" of
 PoolSubpage (#10988)

Motivation:

when customer need large of 'byteBuf.capacity' in [7168, 8192], the size of 'chunk.subpages' may be inflated when large of byteBuf be released, not consistent with other 'byteBuf.capacity'

Modification:

when maxNumElems == 1 need consider remove from pool

Result:

Fixes #10896.

Co-authored-by: zxingy <zxingy@servyou.com.cn>
---
 .../java/io/netty/buffer/PoolSubpage.java     |  8 +++++-
 .../buffer/PooledByteBufAllocatorTest.java    | 27 +++++++++++++++++--
 2 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/buffer/src/main/java/io/netty/buffer/PoolSubpage.java b/buffer/src/main/java/io/netty/buffer/PoolSubpage.java
index 26880ab1bb..62e905c4d0 100644
--- a/buffer/src/main/java/io/netty/buffer/PoolSubpage.java
+++ b/buffer/src/main/java/io/netty/buffer/PoolSubpage.java
@@ -115,7 +115,13 @@ final class PoolSubpage<T> implements PoolSubpageMetric {
 
         if (numAvail ++ == 0) {
             addToPool(head);
-            return true;
+            /* When maxNumElems == 1, the maximum numAvail is also 1.
+             * Each of these PoolSubpages will go in here when they do free operation.
+             * If they return true directly from here, then the rest of the code will be unreachable
+             * and they will not actually be recycled. So return true only on maxNumElems > 1. */
+            if (maxNumElems > 1) {
+                return true;
+            }
         }
 
         if (numAvail != maxNumElems) {
diff --git a/buffer/src/test/java/io/netty/buffer/PooledByteBufAllocatorTest.java b/buffer/src/test/java/io/netty/buffer/PooledByteBufAllocatorTest.java
index 3af459ad93..84986a314c 100644
--- a/buffer/src/test/java/io/netty/buffer/PooledByteBufAllocatorTest.java
+++ b/buffer/src/test/java/io/netty/buffer/PooledByteBufAllocatorTest.java
@@ -622,8 +622,8 @@ public class PooledByteBufAllocatorTest extends AbstractByteBufAllocatorTest<Poo
     }
 
     @SuppressWarnings("unchecked")
-    private static PooledByteBuf<ByteBuffer> unwrapIfNeeded(ByteBuf buf) {
-        return (PooledByteBuf<ByteBuffer>) (buf instanceof PooledByteBuf ? buf : buf.unwrap());
+    private static <T> PooledByteBuf<T> unwrapIfNeeded(ByteBuf buf) {
+        return (PooledByteBuf<T>) (buf instanceof PooledByteBuf ? buf : buf.unwrap());
     }
 
     @Test
@@ -651,4 +651,27 @@ public class PooledByteBufAllocatorTest extends AbstractByteBufAllocatorTest<Poo
         assertEquals(3, allocator.metric().directArenas().get(0).numNormalAllocations());
         buffer.release();
     }
+
+    @Test
+    public void testNormalPoolSubpageRelease() {
+        // 16 < elemSize <= 7168 or 8192 < elemSize <= 28672, 1 < subpage.maxNumElems <= 256
+        // 7168 <= elemSize <= 8192, subpage.maxNumElems == 1
+        int elemSize = 8192;
+        int length = 1024;
+        ByteBuf[] byteBufs = new ByteBuf[length];
+        final PooledByteBufAllocator allocator = new PooledByteBufAllocator(false, 32, 32, 8192, 11, 256, 64, false, 0);
+
+        for (int i = 0; i < length; i++) {
+            byteBufs[i] = allocator.heapBuffer(elemSize, elemSize);
+        }
+        PoolChunk<Object> chunk = unwrapIfNeeded(byteBufs[0]).chunk;
+
+        int beforeFreeBytes = chunk.freeBytes();
+        for (int i = 0; i < length; i++) {
+            byteBufs[i].release();
+        }
+        int afterFreeBytes = chunk.freeBytes();
+
+        assertTrue(beforeFreeBytes < afterFreeBytes);
+    }
 }