Skip to content

Commit

Permalink
Fix hierynomus#777: Don't request excessive read-ahead packets in Rem…
Browse files Browse the repository at this point in the history
…oteFile

Due to a bug in logic introduced by hierynomus#769, RemoteFile.ReadAheadRemoteFileInputStream started to send new read ahead requests for file parts that had already been requested.

Every call to read() asked the server to send parts of the file from the point which is already downloaded. Instead, it should have asked to send parts after the last requested part. This commit adds exactly this logic.

The bug didn't cause content corruption. It only affected performance, both on servers and on clients.
  • Loading branch information
vladimirlagunov committed Apr 6, 2022
1 parent e9cb909 commit 46587d6
Showing 1 changed file with 11 additions and 2 deletions.
13 changes: 11 additions & 2 deletions src/main/java/net/schmizz/sshj/sftp/RemoteFile.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.util.ArrayDeque;
import java.util.Deque;
import java.util.LinkedList;
import java.util.Queue;
import java.util.concurrent.TimeUnit;
Expand Down Expand Up @@ -252,7 +254,7 @@ public int getLength() {

private final int maxUnconfirmedReads;
private final long readAheadLimit;
private final Queue<UnconfirmedRead> unconfirmedReads = new LinkedList<>();
private final Deque<UnconfirmedRead> unconfirmedReads = new ArrayDeque<>();

private long currentOffset;
private int maxReadLength = Integer.MAX_VALUE;
Expand Down Expand Up @@ -336,7 +338,14 @@ public int read(byte[] into, int off, int len) throws IOException {
// we also need to go here for len <= 0, because pending may be at
// EOF in which case it would return -1 instead of 0

long requestOffset = currentOffset;
long requestOffset;
if (unconfirmedReads.isEmpty()) {
requestOffset = currentOffset;
}
else {
final UnconfirmedRead lastRequest = unconfirmedReads.getLast();
requestOffset = lastRequest.offset + lastRequest.length;
}
while (unconfirmedReads.size() <= maxUnconfirmedReads) {
// Send read requests as long as there is no EOF and we have not reached the maximum parallelism
int reqLen = Math.min(Math.max(1024, len), maxReadLength);
Expand Down

0 comments on commit 46587d6

Please sign in to comment.