From e7774feb12121bf4e7bb8140f80ec0d1488f70eb Mon Sep 17 00:00:00 2001
From: zzz <zzz@i2pmail.org>
Date: Sun, 20 Apr 2025 12:06:28 -0400
Subject: [PATCH] i2psnark: Remove outstanding requests from pieces when
 connection is replaced

Hopefully fixes rare case where torrents get stuck.
Tested that it doesn't break anything, but was never able to reproduce the issue.

As reported in torrent at http://tracker2.postman.i2p/index.php?view=TorrentDetail&id=85798
Fix adapted and greatly simplified from source code included in that torrent.
Apparently from old trac ticket #691.
Credit: smtorrents

Also:
- Fix place where removePeerFromPieces() was being called twice,
  once directly and once from disconnect(true).
- Fix javadoc for Peer.disconnect()
- Fix log typo
- Add comments

Description from OP of that torrent:

This update now finally fixes the decades old ticket number 691 which prevents torrents to complete from time to time. The bug manifests itself by keeping a couple of outstanding pieces (> 8, so we never get into the endgame) that are marked as requested from a peerID, with a corresponding peer that has long gone. Those pieces are never requested from any other peer. There are consequently more peerIDs than peers connected.

The only place in code, where peers and peer IDs are mixed up, is a reconnect logic that tries to reuse an existing connection while keeping outstanding requests from the old connection, when the other end tries to reconnect.

Keeping old requests is faulty by itself because someone reconnecting to us will definitely have dropped our previous requests, so we must do the same. The fix now always drops previous connections along with all outstanding requests.
---
 apps/i2psnark/java/src/org/klomp/snark/Peer.java         | 8 ++++----
 .../java/src/org/klomp/snark/PeerCoordinator.java        | 9 ++++++---
 2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/apps/i2psnark/java/src/org/klomp/snark/Peer.java b/apps/i2psnark/java/src/org/klomp/snark/Peer.java
index 959ba7fe0..e14f30630 100644
--- a/apps/i2psnark/java/src/org/klomp/snark/Peer.java
+++ b/apps/i2psnark/java/src/org/klomp/snark/Peer.java
@@ -485,10 +485,10 @@ public class Peer implements Comparable<Peer>, BandwidthListener
   }
 
   /**
-   * Disconnects this peer if it was connected.  If deregister is
-   * true, PeerListener.disconnected() will be called when the
-   * connection is completely terminated. Otherwise the connection is
-   * silently terminated.
+   * Disconnects this peer if it was connected.
+   * PeerListener.disconnected() will be called when the
+   * connection is completely terminated.
+   * If deregister is true, partial pieces will be returned.
    */
   public void disconnect(boolean deregister)
   {
diff --git a/apps/i2psnark/java/src/org/klomp/snark/PeerCoordinator.java b/apps/i2psnark/java/src/org/klomp/snark/PeerCoordinator.java
index b4126fb8e..23f88171c 100644
--- a/apps/i2psnark/java/src/org/klomp/snark/PeerCoordinator.java
+++ b/apps/i2psnark/java/src/org/klomp/snark/PeerCoordinator.java
@@ -647,8 +647,8 @@ class PeerCoordinator implements PeerListener, BandwidthListener
 
     while (!removed.isEmpty()) {
         Peer peer = removed.remove(0);
+        // disconnect() calls disconnected() calls removePeerFromPieces()
         peer.disconnect();
-        removePeerFromPieces(peer);
     }
     // delete any saved orphan partial piece
     synchronized (partialPieces) {
@@ -694,7 +694,7 @@ class PeerCoordinator implements PeerListener, BandwidthListener
         if (old != null && old.getInactiveTime() > old.getMaxInactiveTime()) {
             // idle for 8 minutes, kill the old con (32KB/8min = 68B/sec minimum for one block)
             if (_log.shouldLog(Log.WARN))
-              _log.warn("Remomving old peer: " + peer + ": " + old + ", inactive for " + old.getInactiveTime());
+              _log.warn("Removing old peer: " + peer + ": " + old + ", inactive for " + old.getInactiveTime());
             peers.remove(old);
             toDisconnect = old;
             old = null;
@@ -747,7 +747,10 @@ class PeerCoordinator implements PeerListener, BandwidthListener
           }
       }
     if (toDisconnect != null) {
-        toDisconnect.disconnect(false);
+        // ensure partial pieces are returned
+        toDisconnect.disconnect(true);
+        // disconnect() calls disconnected() but will not call removePeerFromPieces()
+        // because it was removed from peers above
         removePeerFromPieces(toDisconnect);
     }
   }