[#5597] Not try to double release empty buffer in Unpooled.wrappedBuffer(...)

Motivation:

When Unpooled.wrappedBuffer(...) is called with an array of ByteBuf with length >= 2 and the first ByteBuf is not readable it will result in double releasing of these empty buffers when release() is called on the returned buffer.

Modifications:

- Ensure we only wrap readable buffers.
- Add unit test

Result:

No double release of buffers.

buffer/src/main/java/io/netty/buffer/CompositeByteBuf.java

@@ -65,6 +65,11 @@ public class CompositeByteBuf extends AbstractReferenceCountedByteBuf implements
     }
 
     public CompositeByteBuf(ByteBufAllocator alloc, boolean direct, int maxNumComponents, ByteBuf... buffers) {
+        this(alloc, direct, maxNumComponents, buffers, 0, buffers.length);
+    }
+
+    CompositeByteBuf(
+            ByteBufAllocator alloc, boolean direct, int maxNumComponents, ByteBuf[] buffers, int offset, int len) {
         super(Integer.MAX_VALUE);
         if (alloc == null) {
             throw new NullPointerException("alloc");
@@ -79,7 +84,7 @@ public class CompositeByteBuf extends AbstractReferenceCountedByteBuf implements
         this.maxNumComponents = maxNumComponents;
         components = newList(maxNumComponents);
 
-        addComponents0(false, 0, buffers);
+        addComponents0(false, 0, buffers, offset, len);
         consolidateIfNeeded();
         setIndex(0, capacity());
     }
@@ -202,7 +207,7 @@ public class CompositeByteBuf extends AbstractReferenceCountedByteBuf implements
      * ownership of all {@link ByteBuf} objects is transfered to this {@link CompositeByteBuf}.
      */
     public CompositeByteBuf addComponents(boolean increaseWriterIndex, ByteBuf... buffers) {
-        addComponents0(increaseWriterIndex, components.size(), buffers);
+        addComponents0(increaseWriterIndex, components.size(), buffers, 0, buffers.length);
         consolidateIfNeeded();
         return this;
     }
@@ -294,19 +299,19 @@ public class CompositeByteBuf extends AbstractReferenceCountedByteBuf implements
      * ownership of all {@link ByteBuf} objects is transfered to this {@link CompositeByteBuf}.
      */
     public CompositeByteBuf addComponents(int cIndex, ByteBuf... buffers) {
-        addComponents0(false, cIndex, buffers);
+        addComponents0(false, cIndex, buffers, 0, buffers.length);
         consolidateIfNeeded();
         return this;
     }
 
-    private int addComponents0(boolean increaseWriterIndex, int cIndex, ByteBuf... buffers) {
+    private int addComponents0(boolean increaseWriterIndex, int cIndex, ByteBuf[] buffers, int offset, int len) {
         checkNotNull(buffers, "buffers");
-        int i = 0;
+        int i = offset;
         try {
             checkComponentIndex(cIndex);
 
             // No need for consolidation
-            while (i < buffers.length) {
+            while (i < len) {
                 // Increment i now to prepare for the next iteration and prevent a duplicate release (addComponent0
                 // will release if an exception occurs, and we also release in the finally block here).
                 ByteBuf b = buffers[i++];
@@ -321,7 +326,7 @@ public class CompositeByteBuf extends AbstractReferenceCountedByteBuf implements
             }
             return cIndex;
         } finally {
-            for (; i < buffers.length; ++i) {
+            for (; i < len; ++i) {
                 ByteBuf b = buffers[i];
                 if (b != null) {
                     try {
@@ -383,7 +388,7 @@ public class CompositeByteBuf extends AbstractReferenceCountedByteBuf implements
         }
 
         Collection<ByteBuf> col = (Collection<ByteBuf>) buffers;
-        return addComponents0(increaseIndex, cIndex, col.toArray(new ByteBuf[col.size()]));
+        return addComponents0(increaseIndex, cIndex, col.toArray(new ByteBuf[col.size()]), 0 , col.size());
     }
 
     /**

buffer/src/main/java/io/netty/buffer/Unpooled.java

@@ -309,13 +309,14 @@ public final class Unpooled {
             }
             break;
         default:
-            for (ByteBuf b: buffers) {
-                if (b.isReadable()) {
-                    return new CompositeByteBuf(ALLOC, false, maxNumComponents, buffers);
-                } else {
-                    b.release();
+            for (int i = 0; i < buffers.length; i++) {
+                ByteBuf buf = buffers[i];
+                if (buf.isReadable()) {
+                    return new CompositeByteBuf(ALLOC, false, maxNumComponents, buffers, i, buffers.length);
                 }
+                buf.release();
             }
+            break;
         }
         return EMPTY_BUFFER;
     }

buffer/src/test/java/io/netty/buffer/UnpooledTest.java

@@ -628,4 +628,22 @@ public class UnpooledTest {
         ByteBuf buf = freeLater(buffer(8));
         buf.skipBytes(-1);
     }
+
+    // See https://github.com/netty/netty/issues/5597
+    @Test
+    public void testWrapByteBufArrayStartsWithNonReadable() {
+        ByteBuf buffer1 = buffer(8);
+        ByteBuf buffer2 = buffer(8).writeZero(8); // Ensure the ByteBuf is readable.
+        ByteBuf buffer3 = buffer(8);
+        ByteBuf buffer4 = buffer(8).writeZero(8); // Ensure the ByteBuf is readable.
+
+        ByteBuf wrapped = wrappedBuffer(buffer1, buffer2, buffer3, buffer4);
+        assertEquals(16, wrapped.readableBytes());
+        assertTrue(wrapped.release());
+        assertEquals(0, buffer1.refCnt());
+        assertEquals(0, buffer2.refCnt());
+        assertEquals(0, buffer3.refCnt());
+        assertEquals(0, buffer4.refCnt());
+        assertEquals(0, wrapped.refCnt());
+    }
 }