* Tunnel TestJob:

- Don't continue testing after the pool is dead
    - Tweak test intervals
  * TunnelPool: Make more methods package private

router/java/src/net/i2p/router/tunnel/pool/TestJob.java

@@ -23,6 +23,10 @@ import net.i2p.stat.Rate;
 import net.i2p.stat.RateStat;
 import net.i2p.util.Log;
 
+/**
+ *  Repeatedly test a single tunnel for its entire lifetime,
+ *  or until the pool is shut down or removed from the client manager.
+ */
 class TestJob extends JobImpl {
     private final Log _log;
     private final TunnelPool _pool;
@@ -35,7 +39,7 @@ class TestJob extends JobImpl {
     private SessionTag _encryptTag;
     
     /** base to randomize the test delay on */
-    private static final int TEST_DELAY = 30*1000;
+    private static final int TEST_DELAY = 40*1000;
     
     public TestJob(RouterContext ctx, PooledTunnelCreatorConfig cfg, TunnelPool pool) {
         super(ctx);
@@ -54,7 +58,7 @@ class TestJob extends JobImpl {
     public String getName() { return "Test tunnel"; }
 
     public void runJob() {
-        if (_pool == null)
+        if (_pool == null || !_pool.isAlive())
             return;
         long lag = getContext().jobQueue().getMaxLag();
         if (lag > 3000) {
@@ -155,6 +159,8 @@ class TestJob extends JobImpl {
     }
     
     public void testSuccessful(int ms) {
+        if (_pool == null || !_pool.isAlive())
+            return;
         getContext().statManager().addRateData("tunnel.testSuccessLength", _cfg.getLength(), 0);
         getContext().statManager().addRateData("tunnel.testSuccessTime", ms, 0);
     
@@ -182,6 +188,8 @@ class TestJob extends JobImpl {
     }
     
     private void testFailed(long timeToFail) {
+        if (_pool == null || !_pool.isAlive())
+            return;
         if (_found) {
             // ok, not really a /success/, but we did find it, even though slowly
             noteSuccess(timeToFail, _outTunnel);
@@ -208,7 +216,7 @@ class TestJob extends JobImpl {
     }
     
     /** randomized time we should wait before testing */
-    private int getDelay() { return TEST_DELAY + getContext().random().nextInt(TEST_DELAY); }
+    private int getDelay() { return TEST_DELAY + getContext().random().nextInt(TEST_DELAY / 3); }
 
     /** how long we allow tests to run for before failing them */
     private int getTestPeriod() {
@@ -234,8 +242,11 @@ class TestJob extends JobImpl {
 
     private void scheduleRetest() { scheduleRetest(false); }
     private void scheduleRetest(boolean asap) {
+        if (_pool == null || !_pool.isAlive())
+            return;
         if (asap) {
-            requeue(getContext().random().nextInt(TEST_DELAY));
+            if (_cfg.getExpiration() > getContext().clock().now() + (60 * 1000))
+                requeue((TEST_DELAY / 4) + getContext().random().nextInt(TEST_DELAY / 4));
         } else {
             int delay = getDelay();
             if (_cfg.getExpiration() > getContext().clock().now() + delay + (3 * getTestPeriod()))

router/java/src/net/i2p/router/tunnel/pool/TunnelPool.java

@@ -23,6 +23,7 @@ import net.i2p.util.Log;
 
 /**
  *  A group of tunnels for the router or a particular client, in a single direction.
+ *  Public only for TunnelRenderer in router console.
  */
 public class TunnelPool {
     private final List<PooledTunnelCreatorConfig> _inProgress = new ArrayList();
@@ -43,7 +44,7 @@ public class TunnelPool {
     private final String _rateName;
     private static final int TUNNEL_LIFETIME = 10*60*1000;
     
-    public TunnelPool(RouterContext ctx, TunnelPoolManager mgr, TunnelPoolSettings settings, TunnelPeerSelector sel) {
+    TunnelPool(RouterContext ctx, TunnelPoolManager mgr, TunnelPoolSettings settings, TunnelPeerSelector sel) {
         _context = ctx;
         _log = ctx.logManager().getLog(TunnelPool.class);
         _manager = mgr;
@@ -59,10 +60,18 @@ public class TunnelPool {
         refreshSettings();
     }
     
-    public void startup() {
+    /**
+     *  Warning, this may be called more than once
+     *  (without an intervening shutdown()) if the
+     *  tunnel is stopped and then restarted by the client manager with the same
+     *  Destination (i.e. for servers or clients w/ persistent key)
+     */
+    void startup() {
         synchronized (_inProgress) {
             _inProgress.clear();
         }
+        if (_log.shouldLog(Log.WARN))
+            _log.warn(toString() + ": Startup() called, was already alive? " + _alive, new Exception());
         _alive = true;
         _started = System.currentTimeMillis();
         _lastRateUpdate = _started;
@@ -84,7 +93,9 @@ public class TunnelPool {
                                new long[] { 5*60*1000l });
     }
     
-    public void shutdown() {
+    void shutdown() {
+        if (_log.shouldLog(Log.WARN))
+            _log.warn(toString() + ": Shutdown called", new Exception());
         _alive = false;
         _lastSelectionPeriod = 0;
         _lastSelected = null;
@@ -132,7 +143,8 @@ public class TunnelPool {
      * and returns it.
      *
      */
-    public TunnelInfo selectTunnel() { return selectTunnel(true); }
+    TunnelInfo selectTunnel() { return selectTunnel(true); }
+
     private TunnelInfo selectTunnel(boolean allowRecurseOnFail) {
         boolean avoidZeroHop = ((getSettings().getLength() + getSettings().getLengthVariance()) > 0);
         
@@ -239,7 +251,7 @@ public class TunnelPool {
      * Do we really need more fallbacks?
      * Used to prevent a zillion of them
      */
-    public boolean needFallback() {
+    boolean needFallback() {
         int needed = _settings.getBackupQuantity() + _settings.getQuantity();
         int fallbacks = 0;
         synchronized (_tunnels) {
@@ -259,7 +271,8 @@ public class TunnelPool {
     int getTunnelCount() { synchronized (_tunnels) { return _tunnels.size(); } }
     
     public TunnelPoolSettings getSettings() { return _settings; }
-    public void setSettings(TunnelPoolSettings settings) { 
+
+    void setSettings(TunnelPoolSettings settings) { 
         _settings = settings; 
         if (_settings != null) {
             if (_log.shouldLog(Log.INFO))
@@ -267,8 +280,19 @@ public class TunnelPool {
             _manager.getExecutor().repoll(); // in case we need more
         }
     }
-    public TunnelPeerSelector getSelector() { return _peerSelector; }
+
-    public boolean isAlive() { return _alive; }
+    /**
+     *  Is this pool running AND either exploratory, or tracked by the client manager?
+     *  A pool will be alive but not tracked after the client manager removes it
+     *  but before all the tunnels have expired.
+     */
+    public boolean isAlive() {
+        return _alive &&
+               (_settings.isExploratory() ||
+                (_settings.getDestination() != null &&
+                 _context.clientManager().isLocal(_settings.getDestination())));
+    }
+
     /** duplicate of getTunnelCount(), let's pick one */
     public int size() { 
         synchronized (_tunnels) {
@@ -336,17 +360,14 @@ public class TunnelPool {
             }
         }
     
-        boolean connected = true;
+        if (getTunnelCount() <= 0 && !isAlive()) {
-        if ( (_settings.getDestination() != null) && (!_context.clientManager().isLocal(_settings.getDestination())) )
+            // this calls both our shutdown() and the other one (inbound/outbound)
-            connected = false;
-        if ( (getTunnelCount() <= 0) && (!connected) ) {
             _manager.removeTunnels(_settings.getDestination());
-            return;
         }
     }
 
     /** This may be called multiple times from TestJob */
-    public void tunnelFailed(PooledTunnelCreatorConfig cfg) {
+    void tunnelFailed(PooledTunnelCreatorConfig cfg) {
         if (_log.shouldLog(Log.WARN))
             _log.warn(toString() + ": Tunnel failed: " + cfg);
         LeaseSet ls = null;
@@ -574,9 +595,8 @@ public class TunnelPool {
      * the countHowManyToBuild function below)
      *
      */
-    public int countHowManyToBuild() {
+    int countHowManyToBuild() {
-        if (_settings.getDestination() != null) {
+        if (!isAlive()) {
-            if (!_context.clientManager().isLocal(_settings.getDestination()))
                 return 0;
         }
         int wanted = getSettings().getBackupQuantity() + getSettings().getQuantity();

router/java/src/net/i2p/router/tunnel/pool/TunnelPoolManager.java

@@ -276,9 +276,9 @@ public class TunnelPoolManager implements TunnelManagerFacade {
      *  Do not use to change settings.
      */
     public void buildTunnels(Destination client, ClientTunnelSettings settings) {
-        if (_log.shouldLog(Log.DEBUG))
-            _log.debug("Building tunnels for the client " + client.calculateHash().toBase64() + ": " + settings);
         Hash dest = client.calculateHash();
+        if (_log.shouldLog(Log.DEBUG))
+            _log.debug("Building tunnels for the client " + dest + ": " + settings);
         settings.getInboundSettings().setDestination(dest);
         settings.getOutboundSettings().setDestination(dest);
         TunnelPool inbound = null;
@@ -311,7 +311,7 @@ public class TunnelPoolManager implements TunnelManagerFacade {
     
     
     private static class DelayedStartup implements SimpleTimer.TimedEvent {
-        private TunnelPool pool;
+        private final TunnelPool pool;
 
         public DelayedStartup(TunnelPool p) {
             this.pool = p;
@@ -322,9 +322,14 @@ public class TunnelPoolManager implements TunnelManagerFacade {
         }
     }
 
-    /** synch with buildTunnels() above */
+    /**
+     *  This will be called twice, once by the inbound and once by the outbound pool.
+     *  Synched with buildTunnels() above.
+     */
     public synchronized void removeTunnels(Hash destination) {
         if (destination == null) return;
+        if (_log.shouldLog(Log.DEBUG))
+            _log.debug("Removing tunnels for the client " + destination);
         if (_context.clientManager().isLocal(destination)) {
             if (_log.shouldLog(Log.CRIT))
                 _log.log(Log.CRIT, "wtf, why are you removing the pool for " + destination.toBase64(), new Exception("i did it"));