Fix memory leak in AbstractDiskHttpData when CompositeByteBuf is used (#10360)

Motivation:

AbstractDiskHttpData may cause a memory leak when a CompositeByteBuf is used. This happened because we may call copy() but actually never release the newly created ByteBuf.

Modifications:

- Remove copy() call and just use ByteBuf.getBytes(...) which will internally handle the writing to the FileChannel without any extra copies that need to be released later on.
- Add unit test

Result:

Fixes https://github.com/netty/netty/issues/10354
This commit is contained in:
Norman Maurer 2020-06-22 13:54:20 +02:00 committed by GitHub
parent 4dc6764d7b
commit 48938e2d99
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 38 additions and 4 deletions

View File

@ -156,7 +156,6 @@ public abstract class AbstractDiskHttpData extends AbstractHttpData {
throw new IOException("Out of size: " + (size + localsize) +
" > " + definedSize);
}
ByteBuffer byteBuffer = buffer.nioBufferCount() == 1 ? buffer.nioBuffer() : buffer.copy().nioBuffer();
int written = 0;
if (file == null) {
file = tempFile();
@ -165,9 +164,7 @@ public abstract class AbstractDiskHttpData extends AbstractHttpData {
RandomAccessFile accessFile = new RandomAccessFile(file, "rw");
fileChannel = accessFile.getChannel();
}
while (written < localsize) {
written += fileChannel.write(byteBuffer);
}
buffer.getBytes(buffer.readerIndex(), fileChannel, fileChannel.position(), localsize);
size += localsize;
buffer.readerIndex(buffer.readerIndex() + written);
} finally {

View File

@ -17,6 +17,7 @@ package io.netty.handler.codec.http.multipart;
import io.netty.buffer.ByteBuf;
import io.netty.buffer.ByteBufInputStream;
import io.netty.buffer.ByteBufUtil;
import io.netty.buffer.Unpooled;
import io.netty.util.CharsetUtil;
import io.netty.util.internal.PlatformDependent;
@ -182,6 +183,42 @@ public class DiskFileUploadTest {
}
}
@Test
public void testAddContentFromByteBuf() throws Exception {
testAddContentFromByteBuf0(false);
}
@Test
public void testAddContentFromCompositeByteBuf() throws Exception {
testAddContentFromByteBuf0(true);
}
private static void testAddContentFromByteBuf0(boolean composite) throws Exception {
DiskFileUpload f1 = new DiskFileUpload("file3", "file3", "application/json", null, null, 0);
try {
byte[] bytes = new byte[4096];
PlatformDependent.threadLocalRandom().nextBytes(bytes);
final ByteBuf buffer;
if (composite) {
buffer = Unpooled.compositeBuffer()
.addComponent(true, Unpooled.wrappedBuffer(bytes, 0 , bytes.length / 2))
.addComponent(true, Unpooled.wrappedBuffer(bytes, bytes.length / 2, bytes.length / 2));
} else {
buffer = Unpooled.wrappedBuffer(bytes);
}
f1.addContent(buffer, true);
ByteBuf buf = f1.getByteBuf();
assertEquals(buf.readerIndex(), 0);
assertEquals(buf.writerIndex(), bytes.length);
assertArrayEquals(bytes, ByteBufUtil.getBytes(buf));
} finally {
//release the ByteBuf
f1.delete();
}
}
private static byte[] doReadFile(File file, int maxRead) throws Exception {
FileInputStream fis = new FileInputStream(file);
try {