diff --git a/apps/i2psnark/java/src/org/klomp/snark/BitField.java b/apps/i2psnark/java/src/org/klomp/snark/BitField.java index cb147fb268..f724f71d73 100644 --- a/apps/i2psnark/java/src/org/klomp/snark/BitField.java +++ b/apps/i2psnark/java/src/org/klomp/snark/BitField.java @@ -107,6 +107,27 @@ public class BitField } } + /** + * Sets the given bit to false. + * + * @exception IndexOutOfBoundsException if bit is smaller then zero + * bigger then size (inclusive). + * @since 0.9.22 + */ + public void clear(int bit) + { + if (bit < 0 || bit >= size) + throw new IndexOutOfBoundsException(Integer.toString(bit)); + int index = bit/8; + int mask = 128 >> (bit % 8); + synchronized(this) { + if ((bitfield[index] & mask) != 0) { + count--; + bitfield[index] &= ~mask; + } + } + } + /** * Sets all bits to true. * diff --git a/apps/i2psnark/java/src/org/klomp/snark/PartialPiece.java b/apps/i2psnark/java/src/org/klomp/snark/PartialPiece.java index bcf0aefd0b..6cf35836db 100644 --- a/apps/i2psnark/java/src/org/klomp/snark/PartialPiece.java +++ b/apps/i2psnark/java/src/org/klomp/snark/PartialPiece.java @@ -108,7 +108,8 @@ class PartialPiece implements Comparable { /** * Convert this PartialPiece to a request for the next chunk. - * Used by PeerState only. + * Used by PeerState only. This depends on the downloaded value + * as set by setDownloaded() or read(). */ public Request getRequest() { @@ -128,14 +129,16 @@ class PartialPiece implements Comparable { } /** - * How many bytes are good - only valid by setDownloaded() + * How many bytes are good - as set by setDownloaded() or read() */ public int getDownloaded() { return this.off; } /** - * Call this before returning a PartialPiece to the PeerCoordinator + * Call this if necessary before returning a PartialPiece to the PeerCoordinator. + * We do not use a bitmap to track individual chunks received. + * Any chunks after a 'hole' will be lost. * @since 0.9.1 */ public void setDownloaded(int offset) { @@ -191,11 +194,20 @@ class PartialPiece implements Comparable { /** * Blocking. + * If offset matches the previous downloaded amount + * (as set by a previous call to read() or setDownlaoded()), + * the downloaded amount will be incremented by len. + * * @since 0.9.1 */ - public void read(DataInputStream din, int off, int len) throws IOException { + public void read(DataInputStream din, int offset, int len) throws IOException { if (bs != null) { - din.readFully(bs, off, len); + din.readFully(bs, offset, len); + synchronized (this) { + // only works for in-order chunks + if (this.off == offset) + this.off += len; + } } else { // read in fully before synching on raf ByteArray ba; @@ -211,8 +223,11 @@ class PartialPiece implements Comparable { synchronized (this) { if (raf == null) createTemp(); - raf.seek(off); + raf.seek(offset); raf.write(tmp); + // only works for in-order chunks + if (this.off == offset) + this.off += len; } if (ba != null) _cache.release(ba, false); diff --git a/apps/i2psnark/java/src/org/klomp/snark/PeerConnectionOut.java b/apps/i2psnark/java/src/org/klomp/snark/PeerConnectionOut.java index 51272e8a67..314b534bce 100644 --- a/apps/i2psnark/java/src/org/klomp/snark/PeerConnectionOut.java +++ b/apps/i2psnark/java/src/org/klomp/snark/PeerConnectionOut.java @@ -170,6 +170,8 @@ class PeerConnectionOut implements Runnable lastSent = System.currentTimeMillis(); // Remove all piece messages after sending a choke message. + // FiXME this causes REJECT messages to be sent before sending the CHOKE; + // BEP 6 recommends sending them after. if (m.type == Message.CHOKE) removeMessage(Message.PIECE); diff --git a/apps/i2psnark/java/src/org/klomp/snark/PeerCoordinator.java b/apps/i2psnark/java/src/org/klomp/snark/PeerCoordinator.java index 7f5e8a4f03..b347b54488 100644 --- a/apps/i2psnark/java/src/org/klomp/snark/PeerCoordinator.java +++ b/apps/i2psnark/java/src/org/klomp/snark/PeerCoordinator.java @@ -920,6 +920,7 @@ class PeerCoordinator implements PeerListener * Returns a byte array containing the requested piece or null of * the piece is unknown. * + * @return bytes or null for errors such as not having the piece yet * @throws RuntimeException on IOE getting the data */ public ByteArray gotRequest(Peer peer, int piece, int off, int len) @@ -1010,9 +1011,21 @@ class PeerCoordinator implements PeerListener } else { + // so we will try again + markUnrequested(peer, piece); + // just in case + removePartialPiece(piece); // Oops. We didn't actually download this then... :( downloaded.addAndGet(0 - metainfo.getPieceLength(piece)); - _log.warn("Got BAD piece " + piece + "/" + metainfo.getPieces() + " from " + peer + " for " + metainfo.getName()); + // Mark this peer as not having the piece. PeerState will update its bitfield. + for (Piece pc : wantedPieces) { + if (pc.getId() == piece) { + pc.removePeer(peer); + break; + } + } + if (_log.shouldWarn()) + _log.warn("Got BAD piece " + piece + "/" + metainfo.getPieces() + " from " + peer + " for " + metainfo.getName()); return false; // No need to announce BAD piece to peers. } } @@ -1141,8 +1154,9 @@ class PeerCoordinator implements PeerListener * * Also mark the piece unrequested if this peer was the only one. * - * @param peer partials, must include the zero-offset (empty) ones too - * No dup pieces, piece.setDownloaded() must be set + * @param peer partials, must include the zero-offset (empty) ones too. + * No dup pieces, piece.setDownloaded() must be set. + * len field in Requests is ignored. * @since 0.8.2 */ public void savePartialPieces(Peer peer, List partials) diff --git a/apps/i2psnark/java/src/org/klomp/snark/PeerState.java b/apps/i2psnark/java/src/org/klomp/snark/PeerState.java index cf29c415fb..23cbe56907 100644 --- a/apps/i2psnark/java/src/org/klomp/snark/PeerState.java +++ b/apps/i2psnark/java/src/org/klomp/snark/PeerState.java @@ -21,6 +21,7 @@ package org.klomp.snark; import java.util.ArrayList; +import java.util.Collections; import java.util.HashSet; import java.util.Iterator; import java.util.List; @@ -238,6 +239,8 @@ class PeerState implements DataLoader } // Sanity check + // There is no check here that we actually have the piece; + // this will be caught in loadData() below if (piece < 0 || piece >= metainfo.getPieces() || begin < 0 @@ -251,6 +254,8 @@ class PeerState implements DataLoader + ", " + begin + ", " + length + "' message from " + peer); + if (peer.supportsFast()) + out.sendReject(piece, begin, length); return; } @@ -281,7 +286,8 @@ class PeerState implements DataLoader /** * This is the callback that PeerConnectionOut calls * - * @return bytes or null for errors + * @return bytes or null for errors such as not having the piece yet + * @throws RuntimeException on IOE getting the data * @since 0.8.2 */ public ByteArray loadData(int piece, int begin, int length) { @@ -291,6 +297,8 @@ class PeerState implements DataLoader // XXX - Protocol error-> diconnect? if (_log.shouldLog(Log.WARN)) _log.warn("Got request for unknown piece: " + piece); + if (peer.supportsFast()) + out.sendReject(piece, begin, length); return null; } @@ -303,6 +311,8 @@ class PeerState implements DataLoader + ", " + begin + ", " + length + "' message from " + peer); + if (peer.supportsFast()) + out.sendReject(piece, begin, length); return null; } @@ -360,6 +370,11 @@ class PeerState implements DataLoader { if (_log.shouldLog(Log.WARN)) _log.warn("Got BAD " + req.getPiece() + " from " + peer); + synchronized(this) { + // so we don't ask again + if (bitfield != null) + bitfield.clear(req.getPiece()); + } } } @@ -493,7 +508,12 @@ class PeerState implements DataLoader for (Integer p : pcs) { Request req = getLowestOutstandingRequest(p.intValue()); if (req != null) { - req.getPartialPiece().setDownloaded(req.off); + PartialPiece pp = req.getPartialPiece(); + synchronized(pp) { + int dl = pp.getDownloaded(); + if (req.off != dl) + req = new Request(pp, dl, 1); + } rv.add(req); } } @@ -598,6 +618,13 @@ class PeerState implements DataLoader /** * BEP 6 + * If the peer rejects lower chunks but not higher ones, thus creating holes, + * we won't figure it out and the piece will fail, since we don't currently + * keep a chunk bitmap in PartialPiece. + * As long as the peer rejects all the chunks, or rejects only the last chunks, + * no holes are created and we will be fine. The reject messages may be in any order, + * just don't make a hole when it's over. + * * @since 0.9.21 */ void rejectMessage(int piece, int begin, int length) { @@ -605,10 +632,34 @@ class PeerState implements DataLoader _log.info("Got reject(" + piece + ',' + begin + ',' + length + ") from " + peer); out.cancelRequest(piece, begin, length); synchronized(this) { + Request deletedRequest = null; + // for this piece only + boolean haveMoreRequests = false; for (Iterator iter = outstandingRequests.iterator(); iter.hasNext(); ) { Request req = iter.next(); - if (req.getPiece() == piece && req.off == begin && req.len == length) - iter.remove(); + if (req.getPiece() == piece) { + if (req.off == begin && req.len == length) { + iter.remove(); + deletedRequest = req; + } else { + haveMoreRequests = true; + } + } + } + if (deletedRequest != null && !haveMoreRequests) { + // We must return the piece to the coordinator + // Create a new fake request so we can set the offset correctly + PartialPiece pp = deletedRequest.getPartialPiece(); + int downloaded = pp.getDownloaded(); + Request req; + if (deletedRequest.off == downloaded) + req = deletedRequest; + else + req = new Request(pp, downloaded, 1); + List pcs = Collections.singletonList(req); + listener.savePartialPieces(this.peer, pcs); + if (_log.shouldWarn()) + _log.warn("Returned to coord. w/ offset " + pp.getDownloaded() + " due to reject(" + piece + ',' + begin + ',' + length + ") from " + peer); } if (lastRequest != null && lastRequest.getPiece() == piece && lastRequest.off == begin && lastRequest.len == length) diff --git a/apps/i2psnark/java/src/org/klomp/snark/Request.java b/apps/i2psnark/java/src/org/klomp/snark/Request.java index d6a621b1f2..fddbb2b536 100644 --- a/apps/i2psnark/java/src/org/klomp/snark/Request.java +++ b/apps/i2psnark/java/src/org/klomp/snark/Request.java @@ -43,13 +43,13 @@ class Request */ Request(PartialPiece piece, int off, int len) { - // Sanity check - if (off < 0 || len <= 0 || off + len > piece.getLength()) - throw new IndexOutOfBoundsException("Illegal Request " + toString()); - this.piece = piece; this.off = off; this.len = len; + + // Sanity check + if (off < 0 || len <= 0 || off + len > piece.getLength()) + throw new IndexOutOfBoundsException("Illegal Request " + toString()); } /** diff --git a/history.txt b/history.txt index 2cb9a8432c..ab376a4a9b 100644 --- a/history.txt +++ b/history.txt @@ -1,3 +1,17 @@ +2015-08-25 zzz + * i2psnark: + - Return partial piece to coordinator after reject + - Fix tracking of downloaded portion of piece after reject + - Send reject on receipt of bad request + - Mark piece unrequested after receiving bad data, so it + will be requested again, but not from the same peer + - Fix NPE in Request constructor on error + - Fix stuck before completion due to reject handling (ticket #1633) + +2015-08-02 zzz + * Console: Fix SSL excluded ciphers (thx lazyg) + * SU3File: Add keystore password command line option + * 2015-07-31 0.9.21 released 2015-07-27 zzz diff --git a/router/java/src/net/i2p/router/RouterVersion.java b/router/java/src/net/i2p/router/RouterVersion.java index 87e5bffca0..282c18b422 100644 --- a/router/java/src/net/i2p/router/RouterVersion.java +++ b/router/java/src/net/i2p/router/RouterVersion.java @@ -18,7 +18,7 @@ public class RouterVersion { /** deprecated */ public final static String ID = "Monotone"; public final static String VERSION = CoreVersion.VERSION; - public final static long BUILD = 1; + public final static long BUILD = 2; /** for example "-test" */ public final static String EXTRA = "";