From a1f23dbdd34362bf85c188e483a57b743355c12b Mon Sep 17 00:00:00 2001 From: feijermu Date: Mon, 27 Apr 2020 13:03:45 +0800 Subject: [PATCH] Fix a potential fd leak in AbstractDiskHttpData.delete (#10212) Motivation: An unexpected IOException may be thrown from `FileChannel.force`. If it happens, the `FileChannel.close` may not be invoked. Modification: Place the `FileChannel.close` in a finally block. Result: Avoid fd leak. --- .../http/multipart/AbstractDiskHttpData.java | 9 ++++++-- .../http/multipart/DiskFileUploadTest.java | 21 +++++++++++++++++++ 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/codec-http/src/main/java/io/netty/handler/codec/http/multipart/AbstractDiskHttpData.java b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/AbstractDiskHttpData.java index 19d2408123..dd191fa830 100644 --- a/codec-http/src/main/java/io/netty/handler/codec/http/multipart/AbstractDiskHttpData.java +++ b/codec-http/src/main/java/io/netty/handler/codec/http/multipart/AbstractDiskHttpData.java @@ -245,9 +245,14 @@ public abstract class AbstractDiskHttpData extends AbstractHttpData { if (fileChannel != null) { try { fileChannel.force(false); - fileChannel.close(); } catch (IOException e) { - logger.warn("Failed to close a file.", e); + logger.warn("Failed to force.", e); + } finally { + try { + fileChannel.close(); + } catch (IOException e) { + logger.warn("Failed to close a file.", e); + } } fileChannel = null; } diff --git a/codec-http/src/test/java/io/netty/handler/codec/http/multipart/DiskFileUploadTest.java b/codec-http/src/test/java/io/netty/handler/codec/http/multipart/DiskFileUploadTest.java index 2c5270b668..410877b70e 100644 --- a/codec-http/src/test/java/io/netty/handler/codec/http/multipart/DiskFileUploadTest.java +++ b/codec-http/src/test/java/io/netty/handler/codec/http/multipart/DiskFileUploadTest.java @@ -29,6 +29,9 @@ import java.io.InputStream; import static org.junit.Assert.assertArrayEquals; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertTrue; public class DiskFileUploadTest { @@ -194,4 +197,22 @@ public class DiskFileUploadTest { fis.close(); } } + + @Test + public void testDelete() throws Exception { + String json = "{\"foo\":\"bar\"}"; + byte[] bytes = json.getBytes(CharsetUtil.UTF_8); + File tmpFile = null; + DiskFileUpload f1 = new DiskFileUpload("file4", "file4", "application/json", null, null, 0); + try { + assertNull(f1.getFile()); + f1.setContent(Unpooled.wrappedBuffer(bytes)); + assertNotNull(tmpFile = f1.getFile()); + } finally { + f1.delete(); + assertNull(f1.getFile()); + assertNotNull(tmpFile); + assertFalse(tmpFile.exists()); + } + } }