Skip to content

Commit 887a71b

Browse files
committed
Fix FileChannel leak when request fails before FileMultipartPart gets written, close AsyncHttpClient#1418
Motivation: We currently eagerly open FileChannel in FileMultipartPart prior to trying to send the request. If we can’t even try to send the body, eg because connection fails, we never close this FileChannel. Modification: Lazily open FileChannel when writing. Result: No more file descriptor leak
1 parent ffdbe47 commit 887a71b

File tree

1 file changed

+19
-13
lines changed

1 file changed

+19
-13
lines changed

client/src/main/java/org/asynchttpclient/request/body/multipart/part/FileMultipartPart.java

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,7 @@
1616
import static org.asynchttpclient.util.MiscUtils.closeSilently;
1717
import io.netty.buffer.ByteBuf;
1818

19-
import java.io.FileInputStream;
20-
import java.io.FileNotFoundException;
21-
import java.io.IOException;
19+
import java.io.*;
2220
import java.nio.channels.FileChannel;
2321
import java.nio.channels.WritableByteChannel;
2422

@@ -27,19 +25,26 @@
2725

2826
public class FileMultipartPart extends FileLikeMultipartPart<FilePart> {
2927

30-
private final FileChannel channel;
28+
private FileChannel channel;
3129
private final long length;
3230
private long position = 0L;
3331

34-
@SuppressWarnings("resource")
32+
private FileChannel getChannel() throws IOException {
33+
if (channel == null) {
34+
channel = new RandomAccessFile(part.getFile(), "r").getChannel();
35+
}
36+
return channel;
37+
}
38+
3539
public FileMultipartPart(FilePart part, byte[] boundary) {
3640
super(part, boundary);
37-
try {
38-
channel = new FileInputStream(part.getFile()).getChannel();
39-
} catch (FileNotFoundException e) {
40-
throw new IllegalArgumentException("File part doesn't exist: " + part.getFile().getAbsolutePath(), e);
41+
File file = part.getFile();
42+
if (!file.exists()) {
43+
throw new IllegalArgumentException("File part doesn't exist: " + file.getAbsolutePath());
44+
} else if (!file.canRead()) {
45+
throw new IllegalArgumentException("File part can't be read: " + file.getAbsolutePath());
4146
}
42-
length = part.getFile().length();
47+
length = file.length();
4348
}
4449

4550
@Override
@@ -50,7 +55,7 @@ protected long getContentLength() {
5055
@Override
5156
protected long transferContentTo(ByteBuf target) throws IOException {
5257
// can return -1 if file is empty or FileChannel was closed
53-
int transferred = target.writeBytes(channel, target.writableBytes());
58+
int transferred = target.writeBytes(getChannel(), target.writableBytes());
5459
if (transferred > 0) {
5560
position += transferred;
5661
}
@@ -66,8 +71,9 @@ protected long transferContentTo(ByteBuf target) throws IOException {
6671
@Override
6772
protected long transferContentTo(WritableByteChannel target) throws IOException {
6873
// WARN: don't use channel.position(), it's always 0 here
69-
// from FileChannel javadoc: "This method does not modify this channel's position."
70-
long transferred = channel.transferTo(position, BodyChunkedInput.DEFAULT_CHUNK_SIZE, target);
74+
// from FileChannel javadoc: "This method does not modify this channel's
75+
// position."
76+
long transferred = getChannel().transferTo(position, BodyChunkedInput.DEFAULT_CHUNK_SIZE, target);
7177
if (transferred > 0) {
7278
position += transferred;
7379
}

0 commit comments

Comments
 (0)