From 3808777c32dfae4f4025c35243ac11b3c3a9e4ac Mon Sep 17 00:00:00 2001 From: feijermu Date: Mon, 6 Apr 2020 16:12:46 +0800 Subject: [PATCH] Close the file in case of an IOException in AbstractMemoryHttpData.renameTo. (#10163) Motivation: An `IOException` may be thrown from `FileChannel.write` or `FileChannel.force`, and cause the fd leak. Modification: Close the file in a finally block. Result: Avoid fd leak. --- .../multipart/AbstractMemoryHttpData.java | 35 +++++++++-------- .../multipart/AbstractMemoryHttpDataTest.java | 38 +++++++++++++++++++ 2 files changed, 58 insertions(+), 15 deletions(-) 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 e53c56ca2a..14a35be57f 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 @@ -235,24 +235,29 @@ public abstract class AbstractMemoryHttpData extends AbstractHttpData { return true; } int length = byteBuf.readableBytes(); - RandomAccessFile accessFile = new RandomAccessFile(dest, "rw"); - FileChannel fileChannel = accessFile.getChannel(); int written = 0; - if (byteBuf.nioBufferCount() == 1) { - ByteBuffer byteBuffer = byteBuf.nioBuffer(); - while (written < length) { - written += fileChannel.write(byteBuffer); - } - } else { - ByteBuffer[] byteBuffers = byteBuf.nioBuffers(); - while (written < length) { - written += fileChannel.write(byteBuffers); + RandomAccessFile accessFile = new RandomAccessFile(dest, "rw"); + try { + FileChannel fileChannel = accessFile.getChannel(); + try { + if (byteBuf.nioBufferCount() == 1) { + ByteBuffer byteBuffer = byteBuf.nioBuffer(); + while (written < length) { + written += fileChannel.write(byteBuffer); + } + } else { + ByteBuffer[] byteBuffers = byteBuf.nioBuffers(); + while (written < length) { + written += fileChannel.write(byteBuffers); + } + } + fileChannel.force(false); + } finally { + fileChannel.close(); } + } finally { + accessFile.close(); } - - fileChannel.force(false); - fileChannel.close(); - accessFile.close(); return written == length; } diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/multipart/AbstractMemoryHttpDataTest.java b/codec-http/src/test/java/io/netty/handler/codec/http/multipart/AbstractMemoryHttpDataTest.java index 0da3b3ca09..2dd9d6fc30 100644 --- a/codec-http/src/test/java/io/netty/handler/codec/http/multipart/AbstractMemoryHttpDataTest.java +++ b/codec-http/src/test/java/io/netty/handler/codec/http/multipart/AbstractMemoryHttpDataTest.java @@ -24,6 +24,7 @@ import org.junit.Test; import java.io.ByteArrayInputStream; import java.io.File; +import java.io.FileInputStream; import java.io.FileOutputStream; import java.nio.charset.Charset; import java.security.SecureRandom; @@ -64,6 +65,43 @@ public class AbstractMemoryHttpDataTest { test.delete(); } } + + @Test + public void testRenameTo() throws Exception { + TestHttpData test = new TestHttpData("test", UTF_8, 0); + try { + File tmpFile = File.createTempFile(UUID.randomUUID().toString(), ".tmp"); + tmpFile.deleteOnExit(); + final int totalByteCount = 4096; + byte[] bytes = new byte[totalByteCount]; + ThreadLocalRandom.current().nextBytes(bytes); + ByteBuf content = Unpooled.wrappedBuffer(bytes); + test.setContent(content); + boolean succ = test.renameTo(tmpFile); + assertTrue(succ); + FileInputStream fis = new FileInputStream(tmpFile); + try { + byte[] buf = new byte[totalByteCount]; + int count = 0; + int offset = 0; + int size = totalByteCount; + while ((count = fis.read(buf, offset, size)) > 0) { + offset += count; + size -= count; + if (offset >= totalByteCount || size <= 0) { + break; + } + } + assertArrayEquals(bytes, buf); + assertEquals(0, fis.available()); + } finally { + fis.close(); + } + } finally { + //release the ByteBuf in AbstractMemoryHttpData + test.delete(); + } + } /** * Provide content into HTTP data with input stream. *