Merge branch 'partial-ack-fix' into 'master'

SSU: Fix partial acks not being sent when there are no 'gaps'.

See merge request i2p-hackers/i2p.i2p!6

router/java/src/net/i2p/router/transport/udp/InboundMessageState.java

@@ -226,7 +226,11 @@ class InboundMessageState implements CDQEntry {
         return _completeSize;
     }
 
-    /** FIXME synch here or PeerState.fetchPartialACKs() */
+    /**
+     *  Only call this if not complete.
+     *  TODO remove this, have InboundMessageState implement ACKBitfield.
+     *  FIXME synch here or PeerState.fetchPartialACKs()
+     */
     public ACKBitfield createACKBitfield() {
         int last = _lastFragment;
         int sz = (last >= 0) ? last + 1 : _fragments.length;
@@ -236,9 +240,12 @@ class InboundMessageState implements CDQEntry {
     /**
      *  A true partial bitfield that is probably not complete.
      *  fragmentCount() will return 64 if unknown.
+     *
+     *  TODO remove this, have InboundMessageState implement ACKBitfield.
      */
     private static final class PartialBitfield implements ACKBitfield {
         private final long _bitfieldMessageId;
+        private final int _fragmentCount;
         private final int _ackCount;
         private final int _highestReceived;
         // bitfield, 1 for acked
@@ -263,6 +270,7 @@ class InboundMessageState implements CDQEntry {
                 }
             }
             _fragmentAcks = acks;
+            _fragmentCount = size;
             _ackCount = ackCount;
             _highestReceived = highestReceived;
         }
@@ -274,7 +282,12 @@ class InboundMessageState implements CDQEntry {
             return 1L << fragment;
         }
 
-        public int fragmentCount() { return _highestReceived + 1; }
+        /**
+         *  Don't use for serialization since it may be unknown;
+         *  use highestReceived() instead.
+         *  @return 64 if unknown
+         */
+        public int fragmentCount() { return _fragmentCount; }
 
         public int ackCount() { return _ackCount; }
 
@@ -288,7 +301,10 @@ class InboundMessageState implements CDQEntry {
             return (_fragmentAcks & mask(fragmentNum)) != 0;
         }
 
-        public boolean receivedComplete() { return _ackCount == _highestReceived + 1; }
+        /**
+         *  @return should always be false
+         */
+        public boolean receivedComplete() { return _ackCount == _fragmentCount; }
         
         @Override
         public String toString() { 

router/java/src/net/i2p/router/transport/udp/OutboundMessageState.java

@@ -259,7 +259,8 @@ class OutboundMessageState implements CDPQEntry {
         int minSendCount = getMinSendCount();
         // Allow only the fragments we've sent the least
         int rv = 0;
-        for (int i = 0; i < _numFragments; i++) {
+        // see below for why we go in reverse order
+        for (int i = _numFragments - 1; i >= 0; i--) {
             if (needsSending(i) && _fragmentSends[i] == minSendCount) {
                 int sz = fragmentSize(i) + overhead;
                 int tot = rv + sz;
@@ -344,7 +345,13 @@ class OutboundMessageState implements CDPQEntry {
             int minSendCount = getMinSendCount();
             int sent = 0;
             int overhead = _peer.fragmentOverhead();
-            for (int i = 0; i < _numFragments; i++) {
+            // Send in reverse order,
+            // receiver bug workaround.
+            // Through 0.9.48, InboundMessageState.PartialBitfield.isComplete() would return true
+            // if consecutive fragments were complete, and we would not receive partial acks.
+            // Also, if we send the last fragment first, receiver can be more efficient
+            // because it knows the size.
+            for (int i = _numFragments - 1; i >= 0; i--) {
                 if (needsSending(i)) {
                     int count = _fragmentSends[i];
                     if (count == minSendCount) {

router/java/src/net/i2p/router/transport/udp/PacketBuilder.java

@@ -364,8 +364,6 @@ class PacketBuilder {
             for (ACKBitfield bf : partialACKsRemaining) {
                 if (partialAcksToSend >= ABSOLUTE_MAX_ACKS)
                     break;  // ack count
-                if (bf.receivedComplete())
-                    continue;
                 // only send what we have to
                 //int acksz = 4 + (bf.fragmentCount() / 7) + 1;
                 int bits = bf.highestReceived() + 1;

router/java/src/net/i2p/router/transport/udp/PeerState.java

@@ -992,8 +992,7 @@ public class PeerState {
         if (states != null) {
             for (InboundMessageState ims : states) {
                 ACKBitfield abf = ims.createACKBitfield();
-                if (!abf.receivedComplete())
-                    rv.add(abf);
+                rv.add(abf);
             }
         }
     }