Repository: netty/netty
Issue: Memory leak in HttpPostMultipartRequestDecoder#loadDataMultipartOptimized

### Expected behavior
No memory leak

### Actual behavior
There is a memory leak in `io.netty.handler.codec.http.multipart.HttpPostMultipartRequestDecoder#loadDataMultipartOptimized` when the size exceeds `maxLimit`.
Or the issue might be related to `io.netty.handler.codec.http.multipart.AbstractMemoryHttpData#addContent` when checking the size limit.

### Minimal yet complete reproducer code (or URL to code)
Apply the patch below and run the command

`mvn test -pl codec-http -Dtest=io.netty.handler.codec.http.multipart.HttpPostMultiPartRequestDecoderTest#testNotBadReleaseBuffersDuringDecodingMemoryFactory`

Then you will be able to see that when `IOException: Size exceed allowed maximum capacity` is thrown the buffer is not released.

```
diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/multipart/HttpPostMultipartRequestDecoder.java b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/HttpPostMultipartRequestDecoder.java
index a8566c0f7f..29106237cb 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/http/multipart/HttpPostMultipartRequestDecoder.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/HttpPostMultipartRequestDecoder.java
@@ -1187,7 +1187,8 @@ public class HttpPostMultipartRequestDecoder implements InterfaceHttpPostRequest
                 try {
                     httpData.addContent(content, false);
                 } catch (IOException e) {
-                    throw new ErrorDataDecoderException(e);
+                    throw new ErrorDataDecoderException("ByteBuf.refCnt() >>> " + content.refCnt(), e);
+                    //throw new ErrorDataDecoderException(e);
                 }
                 undecodedChunk.readerIndex(startReaderIndex);
                 undecodedChunk.writerIndex(startReaderIndex);
diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/multipart/HttpPostMultiPartRequestDecoderTest.java b/codec-http/src/test/java/io/netty/handler/codec/http/multipart/HttpPostMultiPartRequestDecoderTest.java
index fb7e08202d..d339c422a3 100644
--- a/codec-http/src/test/java/io/netty/handler/codec/http/multipart/HttpPostMultiPartRequestDecoderTest.java
+++ b/codec-http/src/test/java/io/netty/handler/codec/http/multipart/HttpPostMultiPartRequestDecoderTest.java
@@ -311,6 +311,7 @@ public class HttpPostMultiPartRequestDecoderTest {
         request.headers().set("content-type", "multipart/form-data; boundary=861fbeab-cd20-470c-9609-d40a0f704466");
         request.headers().set("content-length", nbItems * (prefix1.length() + prefix2.length() + 2 + bytesPerItem)
                 + suffix.length());
+        factory.setMaxLimit(500);
         HttpPostMultipartRequestDecoder decoder = new HttpPostMultipartRequestDecoder(factory, request);
         decoder.setDiscardThreshold(maxMemory);
         for (int rank = 0; rank < nbItems; rank++) {
```

### Netty version
4.1.68.Final-SNAPSHOT

### JVM version (e.g. `java -version`)

### OS version (e.g. `uname -a`)


Issue comments:
@violetagg sounds like a bug... want to provide a PR ?
@normanmaurer yep let me do that

Pull request review comments:
Looks like `diskAttribute.addContent()` may throw below in which case we may not release the `buffer`. I would suggest put a `try-catch(Exception)` in these methods before we assign `buffer` to an instance variable. This will make sure we release for any unexpected errors.
mmm DiskAttribute itself releases?
See 
io.netty.handler.codec.http.multipart.DiskAttribute#addContent
io.netty.handler.codec.http.multipart.AbstractDiskHttpData#addContent 

We need to be careful to not release twice
Yeah but if I read it correctly, the `ByteBuf` passed to it is different than what is used here:

```java
diskAttribute.addContent(((MemoryAttribute) attribute).getByteBuf(), false)
```
Agree, it should be fixed now

Patch that resolved the issue:
diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/multipart/AbstractMemoryHttpData.java b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/AbstractMemoryHttpData.java
index dc8dd02f1fa..f25801bd504 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/http/multipart/AbstractMemoryHttpData.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/AbstractMemoryHttpData.java
@@ -50,8 +50,14 @@ protected AbstractMemoryHttpData(String name, Charset charset, long size) {
     public void setContent(ByteBuf buffer) throws IOException {
         ObjectUtil.checkNotNull(buffer, "buffer");
         long localsize = buffer.readableBytes();
-        checkSize(localsize);
+        try {
+            checkSize(localsize);
+        } catch (IOException e) {
+            buffer.release();
+            throw e;
+        }
         if (definedSize > 0 && definedSize < localsize) {
+            buffer.release();
             throw new IOException("Out of size: " + localsize + " > " +
                     definedSize);
         }
@@ -99,8 +105,14 @@ public void addContent(ByteBuf buffer, boolean last)
             throws IOException {
         if (buffer != null) {
             long localsize = buffer.readableBytes();
-            checkSize(size + localsize);
+            try {
+                checkSize(size + localsize);
+            } catch (IOException e) {
+                buffer.release();
+                throw e;
+            }
             if (definedSize > 0 && definedSize < size + localsize) {
+                buffer.release();
                 throw new IOException("Out of size: " + (size + localsize) +
                         " > " + definedSize);
             }
diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/multipart/DiskAttribute.java b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/DiskAttribute.java
index f4f77f62b2b..6673ac619cb 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/http/multipart/DiskAttribute.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/DiskAttribute.java
@@ -126,7 +126,12 @@ public void setValue(String value) throws IOException {
     @Override
     public void addContent(ByteBuf buffer, boolean last) throws IOException {
         final long newDefinedSize = size + buffer.readableBytes();
-        checkSize(newDefinedSize);
+        try {
+            checkSize(newDefinedSize);
+        } catch (IOException e) {
+            buffer.release();
+            throw e;
+        }
         if (definedSize > 0 && definedSize < newDefinedSize) {
             definedSize = newDefinedSize;
         }
diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/multipart/HttpData.java b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/HttpData.java
index 266e566523a..72ac59c19a1 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/http/multipart/HttpData.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/HttpData.java
@@ -43,12 +43,13 @@ public interface HttpData extends InterfaceHttpData, ByteBufHolder {
 
     /**
      * Check if the new size is not reaching the max limit allowed.
-     * The limit is always computed in term of bytes.
+     * The limit is always computed in terms of bytes.
      */
     void checkSize(long newSize) throws IOException;
 
     /**
      * Set the content from the ChannelBuffer (erase any previous data)
+     * <p>{@link ByteBuf#release()} ownership of {@code buffer} is transferred to this {@link HttpData}.
      *
      * @param buffer
      *            must be not null
@@ -58,6 +59,7 @@ public interface HttpData extends InterfaceHttpData, ByteBufHolder {
 
     /**
      * Add the content from the ChannelBuffer
+     * <p>{@link ByteBuf#release()} ownership of {@code buffer} is transferred to this {@link HttpData}.
      *
      * @param buffer
      *            must be not null except if last is set to False
diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/multipart/MemoryAttribute.java b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/MemoryAttribute.java
index 4f323a94970..ac1db7490b0 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/http/multipart/MemoryAttribute.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/MemoryAttribute.java
@@ -80,7 +80,12 @@ public void setValue(String value) throws IOException {
     @Override
     public void addContent(ByteBuf buffer, boolean last) throws IOException {
         int localsize = buffer.readableBytes();
-        checkSize(size + localsize);
+        try {
+            checkSize(size + localsize);
+        } catch (IOException e) {
+            buffer.release();
+            throw e;
+        }
         if (definedSize > 0 && definedSize < size + localsize) {
             definedSize = size + localsize;
         }
diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/multipart/MixedAttribute.java b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/MixedAttribute.java
index fbd23fd2fba..f148e4e2563 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/http/multipart/MixedAttribute.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/MixedAttribute.java
@@ -122,16 +122,21 @@ public void checkSize(long newSize) throws IOException {
     @Override
     public void addContent(ByteBuf buffer, boolean last) throws IOException {
         if (attribute instanceof MemoryAttribute) {
-            checkSize(attribute.length() + buffer.readableBytes());
-            if (attribute.length() + buffer.readableBytes() > limitSize) {
-                DiskAttribute diskAttribute = new DiskAttribute(attribute
-                        .getName(), attribute.definedLength(), baseDir, deleteOnExit);
-                diskAttribute.setMaxSize(maxSize);
-                if (((MemoryAttribute) attribute).getByteBuf() != null) {
-                    diskAttribute.addContent(((MemoryAttribute) attribute)
-                        .getByteBuf(), false);
+            try {
+                checkSize(attribute.length() + buffer.readableBytes());
+                if (attribute.length() + buffer.readableBytes() > limitSize) {
+                    DiskAttribute diskAttribute = new DiskAttribute(attribute
+                            .getName(), attribute.definedLength(), baseDir, deleteOnExit);
+                    diskAttribute.setMaxSize(maxSize);
+                    if (((MemoryAttribute) attribute).getByteBuf() != null) {
+                        diskAttribute.addContent(((MemoryAttribute) attribute)
+                            .getByteBuf(), false);
+                    }
+                    attribute = diskAttribute;
                 }
-                attribute = diskAttribute;
+            } catch (IOException e) {
+                buffer.release();
+                throw e;
             }
         }
         attribute.addContent(buffer, last);
@@ -199,7 +204,12 @@ public void setCharset(Charset charset) {
 
     @Override
     public void setContent(ByteBuf buffer) throws IOException {
-        checkSize(buffer.readableBytes());
+        try {
+            checkSize(buffer.readableBytes());
+        } catch (IOException e) {
+            buffer.release();
+            throw e;
+        }
         if (buffer.readableBytes() > limitSize) {
             if (attribute instanceof MemoryAttribute) {
                 // change to Disk
diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/multipart/MixedFileUpload.java b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/MixedFileUpload.java
index b5d5e673ce2..547a3639832 100644
--- a/codec-http/src/main/java/io/netty/handler/codec/http/multipart/MixedFileUpload.java
+++ b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/MixedFileUpload.java
@@ -83,22 +83,27 @@ public void checkSize(long newSize) throws IOException {
     public void addContent(ByteBuf buffer, boolean last)
             throws IOException {
         if (fileUpload instanceof MemoryFileUpload) {
-            checkSize(fileUpload.length() + buffer.readableBytes());
-            if (fileUpload.length() + buffer.readableBytes() > limitSize) {
-                DiskFileUpload diskFileUpload = new DiskFileUpload(fileUpload
-                        .getName(), fileUpload.getFilename(), fileUpload
-                        .getContentType(), fileUpload
-                        .getContentTransferEncoding(), fileUpload.getCharset(),
-                        definedSize, baseDir, deleteOnExit);
-                diskFileUpload.setMaxSize(maxSize);
-                ByteBuf data = fileUpload.getByteBuf();
-                if (data != null && data.isReadable()) {
-                    diskFileUpload.addContent(data.retain(), false);
+            try {
+                checkSize(fileUpload.length() + buffer.readableBytes());
+                if (fileUpload.length() + buffer.readableBytes() > limitSize) {
+                    DiskFileUpload diskFileUpload = new DiskFileUpload(fileUpload
+                            .getName(), fileUpload.getFilename(), fileUpload
+                            .getContentType(), fileUpload
+                            .getContentTransferEncoding(), fileUpload.getCharset(),
+                            definedSize, baseDir, deleteOnExit);
+                    diskFileUpload.setMaxSize(maxSize);
+                    ByteBuf data = fileUpload.getByteBuf();
+                    if (data != null && data.isReadable()) {
+                        diskFileUpload.addContent(data.retain(), false);
+                    }
+                    // release old upload
+                    fileUpload.release();
+
+                    fileUpload = diskFileUpload;
                 }
-                // release old upload
-                fileUpload.release();
-
-                fileUpload = diskFileUpload;
+            } catch (IOException e) {
+                buffer.release();
+                throw e;
             }
         }
         fileUpload.addContent(buffer, last);
@@ -181,7 +186,12 @@ public void setCharset(Charset charset) {
 
     @Override
     public void setContent(ByteBuf buffer) throws IOException {
-        checkSize(buffer.readableBytes());
+        try {
+            checkSize(buffer.readableBytes());
+        } catch (IOException e) {
+            buffer.release();
+            throw e;
+        }
         if (buffer.readableBytes() > limitSize) {
             if (fileUpload instanceof MemoryFileUpload) {
                 FileUpload memoryUpload = fileUpload;


Test patch:
diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/multipart/HttpDataTest.java b/codec-http/src/test/java/io/netty/handler/codec/http/multipart/HttpDataTest.java
new file mode 100644
index 00000000000..17f90e70fa3
--- /dev/null
+++ b/codec-http/src/test/java/io/netty/handler/codec/http/multipart/HttpDataTest.java
@@ -0,0 +1,135 @@
+/*
+ * Copyright 2021 The Netty Project
+ *
+ * The Netty Project licenses this file to you under the Apache License,
+ * version 2.0 (the "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at:
+ *
+ *   https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations
+ * under the License.
+ */
+package io.netty.handler.codec.http.multipart;
+
+import io.netty.buffer.ByteBuf;
+import io.netty.buffer.PooledByteBufAllocator;
+import io.netty.util.CharsetUtil;
+import org.assertj.core.api.ThrowableAssert;
+import org.junit.jupiter.api.BeforeAll;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.MethodSource;
+
+import java.io.IOException;
+import java.lang.annotation.ElementType;
+import java.lang.annotation.Retention;
+import java.lang.annotation.RetentionPolicy;
+import java.lang.annotation.Target;
+import java.util.Random;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
+
+class HttpDataTest {
+    private static final byte[] BYTES = new byte[64];
+
+    @Retention(RetentionPolicy.RUNTIME)
+    @Target(ElementType.METHOD)
+    @ParameterizedTest(name = "{displayName}({0})")
+    @MethodSource("data")
+    @interface ParameterizedHttpDataTest {
+    }
+
+    static HttpData[] data() {
+        return new HttpData[]{
+                new MemoryAttribute("test", 10),
+                new MemoryFileUpload("test", "", "text/plain", null, CharsetUtil.UTF_8, 10),
+                new MixedAttribute("test", 10, -1),
+                new MixedFileUpload("test", "", "text/plain", null, CharsetUtil.UTF_8, 10, -1),
+                new DiskAttribute("test", 10),
+                new DiskFileUpload("test", "", "text/plain", null, CharsetUtil.UTF_8, 10)
+        };
+    }
+
+    @BeforeAll
+    static void setUp() {
+        Random rndm = new Random();
+        rndm.nextBytes(BYTES);
+    }
+
+    @ParameterizedHttpDataTest
+    void testAddContentEmptyBuffer(HttpData httpData) throws IOException {
+        ByteBuf content = PooledByteBufAllocator.DEFAULT.buffer();
+        httpData.addContent(content, false);
+        assertThat(content.refCnt()).isEqualTo(0);
+    }
+
+    @Test
+    void testAddContentExceedsDefinedSizeDiskFileUpload() {
+        doTestAddContentExceedsSize(
+                new DiskFileUpload("test", "", "application/json", null, CharsetUtil.UTF_8, 10),
+                "Out of size: 64 > 10");
+    }
+
+    @Test
+    void testAddContentExceedsDefinedSizeMemoryFileUpload() {
+        doTestAddContentExceedsSize(
+                new MemoryFileUpload("test", "", "application/json", null, CharsetUtil.UTF_8, 10),
+                "Out of size: 64 > 10");
+    }
+
+    @ParameterizedHttpDataTest
+    void testAddContentExceedsMaxSize(final HttpData httpData) {
+        httpData.setMaxSize(10);
+        doTestAddContentExceedsSize(httpData, "Size exceed allowed maximum capacity");
+    }
+
+    @ParameterizedHttpDataTest
+    void testSetContentExceedsDefinedSize(final HttpData httpData) {
+        doTestSetContentExceedsSize(httpData, "Out of size: 64 > 10");
+    }
+
+    @ParameterizedHttpDataTest
+    void testSetContentExceedsMaxSize(final HttpData httpData) {
+        httpData.setMaxSize(10);
+        doTestSetContentExceedsSize(httpData, "Size exceed allowed maximum capacity");
+    }
+
+    private static void doTestAddContentExceedsSize(final HttpData httpData, String expectedMessage) {
+        final ByteBuf content = PooledByteBufAllocator.DEFAULT.buffer();
+        content.writeBytes(BYTES);
+
+        assertThatExceptionOfType(IOException.class)
+                .isThrownBy(new ThrowableAssert.ThrowingCallable() {
+
+                    @Override
+                    public void call() throws Throwable {
+                        httpData.addContent(content, false);
+                    }
+                })
+                .withMessage(expectedMessage);
+
+        assertThat(content.refCnt()).isEqualTo(0);
+    }
+
+    private static void doTestSetContentExceedsSize(final HttpData httpData, String expectedMessage) {
+        final ByteBuf content = PooledByteBufAllocator.DEFAULT.buffer();
+        content.writeBytes(BYTES);
+
+        assertThatExceptionOfType(IOException.class)
+                .isThrownBy(new ThrowableAssert.ThrowingCallable() {
+
+                    @Override
+                    public void call() throws Throwable {
+                        httpData.setContent(content);
+                    }
+                })
+                .withMessage(expectedMessage);
+
+        assertThat(content.refCnt()).isEqualTo(0);
+    }
+}