Do not release the in-progress HttpData twice on destroy() (#66)

destroy() released the HttpData that is still being decoded
unconditionally, assuming the decoder is its only owner. It is not: it is
handed out by currentPartialHttpData(), and a tracking factory releases
it on cleanFiles() or when the user cleans the factory directly. Check
refCnt() before releasing it, like for bodyListHttpData.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

Author: yawkat

multipart-vintage/src/main/java/io/netty/contrib/multipart/vintage/HttpPostMultipartRequestDecoder.java

@@ -868,17 +868,22 @@ private FileUpload getFileUpload() {
      */
     @Override
     public void destroy() {
-        // The partially decoded item is owned by this decoder and never handed out. It is not in
-        // bodyListHttpData, and a memory-only factory does not track it, so release it here. Remove it
-        // from the factory first so that cleanFiles() does not release it a second time.
+        // The HttpData that is still being decoded was never added to bodyListHttpData, and a memory based
+        // one is not tracked by the factory either, so release it here. Remove it from the factory first so
+        // that cleanFiles() below does not release it a second time. It might have been released already by
+        // the user (through currentPartialHttpData(), cleanFiles() or by cleaning the factory directly).
         if (currentFileUpload != null) {
             factory.removeHttpDataFromClean(request, currentFileUpload);
-            currentFileUpload.release();
+            if (currentFileUpload.refCnt() > 0) {
+                currentFileUpload.release();
+            }
             currentFileUpload = null;
         }
         if (currentAttribute != null) {
             factory.removeHttpDataFromClean(request, currentAttribute);
-            currentAttribute.release();
+            if (currentAttribute.refCnt() > 0) {
+                currentAttribute.release();
+            }
             currentAttribute = null;
         }
         // Release all data items, including those not yet pulled, only file based items

multipart-vintage/src/main/java/io/netty/contrib/multipart/vintage/HttpPostStandardRequestDecoder.java

@@ -471,12 +471,15 @@ private void parseBodyAttributes() {
      */
     @Override
     public void destroy() {
-        // The partially decoded item is owned by this decoder and never handed out. It is not in
-        // bodyListHttpData, and a memory-only factory does not track it, so release it here. Remove it
-        // from the factory first so that cleanFiles() does not release it a second time.
+        // The HttpData that is still being decoded was never added to bodyListHttpData, and a memory based
+        // one is not tracked by the factory either, so release it here. Remove it from the factory first so
+        // that cleanFiles() below does not release it a second time. It might have been released already by
+        // the user (through currentPartialHttpData(), cleanFiles() or by cleaning the factory directly).
         if (currentAttribute != null) {
             factory.removeHttpDataFromClean(request, currentAttribute);
-            currentAttribute.release();
+            if (currentAttribute.refCnt() > 0) {
+                currentAttribute.release();
+            }
             currentAttribute = null;
         }
         // Release all data items, including those not yet pulled, only file based items

multipart-vintage/src/test/java/io/netty/contrib/multipart/vintage/HttpPostRequestDecoderTest.java

@@ -38,6 +38,7 @@
 import io.netty.handler.codec.http.multipart.Attribute;
 import io.netty.handler.codec.http.multipart.DefaultHttpDataFactory;
 import io.netty.handler.codec.http.multipart.FileUpload;
+import io.netty.handler.codec.http.multipart.HttpDataFactory;
 import io.netty.handler.codec.http.multipart.InterfaceHttpData;
 import io.netty.handler.codec.http.multipart.MemoryAttribute;
 import io.netty.handler.codec.http.multipart.MemoryFileUpload;
@@ -1290,4 +1291,82 @@ public void testBuilderReuseStandardMaxFields() {
             decoder.destroy();
         }
     }
+
+    /**
+     * A decoder with a field that is still being decoded.
+     */
+    private HttpPostRequestDecoder decoderWithPartialData(HttpDataFactory factory, boolean multipart) {
+        String boundary = "be38b42a9ad2713f";
+        HttpRequest req = new DefaultHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/");
+        String body;
+        if (multipart) {
+            req.headers().add(HttpHeaderNames.CONTENT_TYPE, "multipart/form-data; boundary=" + boundary);
+            body = "--" + boundary + "\r\n" +
+                    "Content-Disposition: form-data; name=\"file\"; filename=\"a.txt\"\r\n" +
+                    "Content-Type: text/plain\r\n" +
+                    "\r\n" +
+                    "some partial content";
+        } else {
+            req.headers().add(HttpHeaderNames.CONTENT_TYPE, "application/x-www-form-urlencoded");
+            body = "field=partialvalue";
+        }
+        HttpPostRequestDecoder decoder = this.builder().dataFactory(factory).build(req);
+        decoder.offer(new DefaultHttpContent(Unpooled.copiedBuffer(body, CharsetUtil.UTF_8)));
+        assertNotNull(decoder.currentPartialHttpData());
+        return decoder;
+    }
+
+    private void testDestroyAfterCleanFiles(boolean multipart) {
+        // the default factory tracks the data, so cleanFiles() releases the partial data
+        HttpPostRequestDecoder decoder = decoderWithPartialData(new DefaultHttpDataFactory(), multipart);
+        InterfaceHttpData partial = decoder.currentPartialHttpData();
+        decoder.cleanFiles();
+        assertEquals(0, partial.refCnt());
+        decoder.destroy();
+    }
+
+    @Test
+    public void testDestroyAfterCleanFilesStandardDecoder() {
+        testDestroyAfterCleanFiles(false);
+    }
+
+    @Test
+    public void testDestroyAfterCleanFilesMultipartDecoder() {
+        testDestroyAfterCleanFiles(true);
+    }
+
+    private void testDestroyAfterFactoryCleaned(boolean multipart) {
+        DefaultHttpDataFactory factory = new DefaultHttpDataFactory();
+        HttpPostRequestDecoder decoder = decoderWithPartialData(factory, multipart);
+        InterfaceHttpData partial = decoder.currentPartialHttpData();
+        factory.cleanAllHttpData();
+        assertEquals(0, partial.refCnt());
+        decoder.destroy();
+    }
+
+    @Test
+    public void testDestroyAfterFactoryCleanedStandardDecoder() {
+        testDestroyAfterFactoryCleaned(false);
+    }
+
+    @Test
+    public void testDestroyAfterFactoryCleanedMultipartDecoder() {
+        testDestroyAfterFactoryCleaned(true);
+    }
+
+    private void testDestroyAfterPartialReleased(boolean multipart) {
+        HttpPostRequestDecoder decoder = decoderWithPartialData(new DefaultHttpDataFactory(false), multipart);
+        assertTrue(decoder.currentPartialHttpData().release());
+        decoder.destroy();
+    }
+
+    @Test
+    public void testDestroyAfterPartialReleasedStandardDecoder() {
+        testDestroyAfterPartialReleased(false);
+    }
+
+    @Test
+    public void testDestroyAfterPartialReleasedMultipartDecoder() {
+        testDestroyAfterPartialReleased(true);
+    }
 }