* I2PTunnel: Don't start a tunnel if no valid destinations;

cleanups, logging, and error propagation fixes

apps/i2ptunnel/java/src/net/i2p/i2ptunnel/I2PTunnel.java

@@ -69,6 +69,9 @@ import net.i2p.util.EventDispatcher;
 import net.i2p.util.EventDispatcherImpl;
 import net.i2p.util.Log;
 
+/**
+ *  Todo: Most events are not listened to elsewhere, so error propagation is poor
+ */
 public class I2PTunnel implements Logging, EventDispatcher {
     private Log _log;
     private EventDispatcherImpl _event;
@@ -180,6 +183,7 @@ public class I2PTunnel implements Logging, EventDispatcher {
         }
     }
 
+    /** @return non-null */
     List<I2PSession> getSessions() { 
         synchronized (_sessions) {
             return new ArrayList(_sessions); 
@@ -585,6 +589,9 @@ public class I2PTunnel implements Logging, EventDispatcher {
      * Run the server pointing at the host and port specified using the private i2p
      * destination loaded from the given base64 stream. <p />
      *
+     * Deprecated? Why run a server with a private destination?
+     * Not available from the war GUI
+     *
      * Sets the event "serverTaskId" = Integer(taskId) after the tunnel has been started (or -1 on error)
      * Also sets the event "openServerResult" = "ok" or "error" (displaying "Ready!" on the logger after
      * 'ok').  So, success = serverTaskId != -1 and openServerResult = ok.
@@ -666,6 +673,11 @@ public class I2PTunnel implements Logging, EventDispatcher {
                 _log.error(getPrefix() + "Invalid I2PTunnel config to create a client [" + host + ":"+ port + "]", iae);
                 l.log("Invalid I2PTunnel configuration [" + host + ":" + port + "]");
                 notifyEvent("clientTaskId", Integer.valueOf(-1));
+                // Since nothing listens to TaskID events, use this to propagate the error to TunnelController
+                // Otherwise, the tunnel stays up even though the port is down
+                // This doesn't work for CLI though... and the tunnel doesn't close itself after error,
+                // so this probably leaves the tunnel open if called from the CLI
+                throw iae;
             }
         } else {
             l.log("client <port> <pubkey>[,<pubkey>]|file:<pubkeyfile>[ <sharedClient>] [<privKeyFile>]");
@@ -733,6 +745,11 @@ public class I2PTunnel implements Logging, EventDispatcher {
                 _log.error(getPrefix() + "Invalid I2PTunnel config to create an httpclient [" + host + ":"+ clientPort + "]", iae);
                 l.log("Invalid I2PTunnel configuration [" + host + ":" + clientPort + "]");
                 notifyEvent("httpclientTaskId", Integer.valueOf(-1));
+                // Since nothing listens to TaskID events, use this to propagate the error to TunnelController
+                // Otherwise, the tunnel stays up even though the port is down
+                // This doesn't work for CLI though... and the tunnel doesn't close itself after error,
+                // so this probably leaves the tunnel open if called from the CLI
+                throw iae;
             }
         } else {
             l.log("httpclient <port> [<sharedClient>] [<proxy>]");
@@ -789,7 +806,12 @@ public class I2PTunnel implements Logging, EventDispatcher {
                 task = new I2PTunnelConnectClient(_port, l, ownDest, proxy, (EventDispatcher) this, this);
                 addtask(task);
             } catch (IllegalArgumentException iae) {
-                _log.error(getPrefix() + "Invalid I2PTunnel config to create an httpclient [" + host + ":"+ _port + "]", iae);
+                _log.error(getPrefix() + "Invalid I2PTunnel config to create a connect client [" + host + ":"+ _port + "]", iae);
+                // Since nothing listens to TaskID events, use this to propagate the error to TunnelController
+                // Otherwise, the tunnel stays up even though the port is down
+                // This doesn't work for CLI though... and the tunnel doesn't close itself after error,
+                // so this probably leaves the tunnel open if called from the CLI
+                throw iae;
             }
         } else {
             l.log("connectclient <port> [<sharedClient>] [<proxy>]");
@@ -848,6 +870,11 @@ public class I2PTunnel implements Logging, EventDispatcher {
                 _log.error(getPrefix() + "Invalid I2PTunnel config to create an ircclient [" + host + ":"+ _port + "]", iae);
                 l.log("Invalid I2PTunnel configuration [" + host + ":" + _port + "]");
                 notifyEvent("ircclientTaskId", Integer.valueOf(-1));
+                // Since nothing listens to TaskID events, use this to propagate the error to TunnelController
+                // Otherwise, the tunnel stays up even though the port is down
+                // This doesn't work for CLI though... and the tunnel doesn't close itself after error,
+                // so this probably leaves the tunnel open if called from the CLI
+                throw iae;
             }
         } else {
             l.log("ircclient <port> [<sharedClient> [<privKeyFile>]]");

apps/i2ptunnel/java/src/net/i2p/i2ptunnel/I2PTunnelClient.java

@@ -20,7 +20,7 @@ public class I2PTunnelClient extends I2PTunnelClientBase {
     private static final Log _log = new Log(I2PTunnelClient.class);
 
     /** list of Destination objects that we point at */
-    protected List dests;
+    protected List<Destination> dests;
     private static final long DEFAULT_READ_TIMEOUT = 5*60*1000; // -1
     protected long readTimeout = DEFAULT_READ_TIMEOUT;
 
@@ -55,9 +55,20 @@ public class I2PTunnelClient extends I2PTunnelClientBase {
         }
 
         if (dests.isEmpty()) {
-            l.log("No target destinations found");
+            l.log("No valid target destinations found");
             notifyEvent("openClientResult", "error");
-            return;
+            // Nothing is listening for the above event, so it's useless
+            // Maybe figure out where to put a waitEventValue("openClientResult") ??
+            // In the meantime, let's do this the easy way
+            // Note that b32 dests will often not be resolvable at instantiation time;
+            // a delayed resolution system would be even better.
+
+            // Don't close() here, because it does a removeSession() and then
+            // TunnelController can't acquire() it to release() it.
+            //close(true);
+            // Unfortunately, super() built the whole tunnel before we get here.
+            throw new IllegalArgumentException("No valid target destinations found");
+            //return;
         }
 
         setName(getLocalPort() + " -> " + destinations);
@@ -98,8 +109,8 @@ public class I2PTunnelClient extends I2PTunnelClientBase {
             return null;
         }
         if (size == 1) // skip the rand in the most common case
-            return (Destination)dests.get(0);
+            return dests.get(0);
         int index = I2PAppContext.getGlobalContext().random().nextInt(size);
-        return (Destination)dests.get(index);
+        return dests.get(index);
     }
 }

apps/i2ptunnel/java/src/net/i2p/i2ptunnel/I2PTunnelClientBase.java

@@ -583,6 +583,8 @@ public abstract class I2PTunnelClientBase extends I2PTunnelTask implements Runna
     }
 
     public boolean close(boolean forced) {
+        if (_log.shouldLog(Log.INFO))
+            _log.info("close() called: forced = " + forced + " open = " + open + " sockMgr = " + sockMgr);
         if (!open) return true;
         // FIXME: here we might have to wait quite a long time if
         // there is a connection attempt atm. But without waiting we

apps/i2ptunnel/java/src/net/i2p/i2ptunnel/I2PTunnelIRCClient.java

@@ -17,6 +17,9 @@ import net.i2p.util.EventDispatcher;
 import net.i2p.util.I2PAppThread;
 import net.i2p.util.Log;
 
+/**
+ * Todo: Can we extend I2PTunnelClient instead and remove some duplicated code?
+ */
 public class I2PTunnelIRCClient extends I2PTunnelClientBase implements Runnable {
 
     private static final Log _log = new Log(I2PTunnelIRCClient.class);
@@ -25,7 +28,7 @@ public class I2PTunnelIRCClient extends I2PTunnelClientBase implements Runnable
     private static volatile long __clientId = 0;
     
     /** list of Destination objects that we point at */
-    protected List dests;
+    protected List<Destination> dests;
     private static final long DEFAULT_READ_TIMEOUT = 5*60*1000; // -1
     protected long readTimeout = DEFAULT_READ_TIMEOUT;
 
@@ -47,7 +50,7 @@ public class I2PTunnelIRCClient extends I2PTunnelClientBase implements Runnable
               "IRCHandler " + (++__clientId), tunnel, pkf);
         
         StringTokenizer tok = new StringTokenizer(destinations, ", ");
-        dests = new ArrayList(1);
+        dests = new ArrayList(2);
         while (tok.hasMoreTokens()) {
             String destination = tok.nextToken();
             try {
@@ -64,7 +67,18 @@ public class I2PTunnelIRCClient extends I2PTunnelClientBase implements Runnable
         if (dests.isEmpty()) {
             l.log("No target destinations found");
             notifyEvent("openClientResult", "error");
-            return;
+            // Nothing is listening for the above event, so it's useless
+            // Maybe figure out where to put a waitEventValue("openClientResult") ??
+            // In the meantime, let's do this the easy way
+            // Note that b32 dests will often not be resolvable at instantiation time;
+            // a delayed resolution system would be even better.
+
+            // Don't close() here, because it does a removeSession() and then
+            // TunnelController can't acquire() it to release() it.
+            //close(true);
+            // Unfortunately, super() built the whole tunnel before we get here.
+            throw new IllegalArgumentException("No valid target destinations found");
+            //return;
         }
         
         setName(getLocalPort() + " -> IRCClient");
@@ -109,9 +123,9 @@ public class I2PTunnelIRCClient extends I2PTunnelClientBase implements Runnable
             return null;
         }
         if (size == 1) // skip the rand in the most common case
-            return (Destination)dests.get(0);
+            return dests.get(0);
         int index = I2PAppContext.getGlobalContext().random().nextInt(size);
-        return (Destination)dests.get(index);
+        return dests.get(index);
     }
 
     /*************************************************************************

apps/i2ptunnel/java/src/net/i2p/i2ptunnel/TunnelController.java

@@ -29,8 +29,8 @@ public class TunnelController implements Logging {
     private Log _log;
     private Properties _config;
     private I2PTunnel _tunnel;
-    private List _messages;
+    private List<String> _messages;
-    private List _sessions;
+    private List<I2PSession> _sessions;
     private boolean _running;
     private boolean _starting;
     
@@ -120,6 +120,9 @@ public class TunnelController implements Logging {
         } catch (Exception e) {
             _log.error("Error starting up the tunnel", e);
             log("Error starting up the tunnel - " + e.getMessage());
+            // if we don't acquire() then the release() in stopTunnel() won't work
+            acquire();
+            stopTunnel();
         }
         _starting = false;
     }
@@ -256,15 +259,18 @@ public class TunnelController implements Logging {
      * closed by some other tunnels
      */
     private void acquire() {
-        List sessions = _tunnel.getSessions();
+        List<I2PSession> sessions = _tunnel.getSessions();
-        if (sessions != null) {
+        if (!sessions.isEmpty()) {
             for (int i = 0; i < sessions.size(); i++) {
-                I2PSession session = (I2PSession)sessions.get(i);
+                I2PSession session = sessions.get(i);
+                if (_log.shouldLog(Log.INFO))
+                    _log.info("Acquiring session " + session);
                 TunnelControllerGroup.getInstance().acquire(this, session);
             }
             _sessions = sessions;
         } else {
-            _log.error("No sessions to acquire?");
+            if (_log.shouldLog(Log.WARN))
+                _log.warn("No sessions to acquire? for " + getName());
         }
     }
     
@@ -273,13 +279,16 @@ public class TunnelController implements Logging {
      * no other tunnels are using them, close them.
      */
     private void release() {
-        if (_sessions != null) {
+        if (_sessions != null && !_sessions.isEmpty()) {
             for (int i = 0; i < _sessions.size(); i++) {
-                I2PSession s = (I2PSession)_sessions.get(i);
+                I2PSession s = _sessions.get(i);
+                if (_log.shouldLog(Log.INFO))
+                    _log.info("Releasing session " + s);
                 TunnelControllerGroup.getInstance().release(this, s);
             }
         } else {
-            _log.error("No sessions to release?");
+            if (_log.shouldLog(Log.WARN))
+                _log.warn("No sessions to release? for " + getName());
         }
     }
     
@@ -597,7 +606,7 @@ public class TunnelController implements Logging {
      *
      * @return list of messages pulled off (each is a String, earliest first)
      */
-    public List clearMessages() { 
+    public List<String> clearMessages() { 
         List rv = null;
         synchronized (this) {
             rv = new ArrayList(_messages);

apps/i2ptunnel/java/src/net/i2p/i2ptunnel/TunnelControllerGroup.java

@@ -25,13 +25,14 @@ import net.i2p.util.Log;
  * Coordinate a set of tunnels within the JVM, loading and storing their config
  * to disk, and building new ones as requested.
  *
+ * Warning - this is a singleton. Todo: fix
  */
 public class TunnelControllerGroup {
-    private Log _log;
+    private final Log _log;
     private static TunnelControllerGroup _instance;
     static final String DEFAULT_CONFIG_FILE = "i2ptunnel.config";
     
-    private List _controllers;
+    private final List<TunnelController> _controllers;
     private String _configFile = DEFAULT_CONFIG_FILE;
     
     /** 
@@ -40,7 +41,7 @@ public class TunnelControllerGroup {
      * no more tunnels are using it)
      *
      */
-    private final Map _sessions;
+    private final Map<I2PSession, Set<TunnelController>> _sessions;
     
     public static TunnelControllerGroup getInstance() { 
         synchronized (TunnelControllerGroup.class) {
@@ -104,7 +105,7 @@ public class TunnelControllerGroup {
     private class StartControllers implements Runnable {
         public void run() {
             for (int i = 0; i < _controllers.size(); i++) {
-                TunnelController controller = (TunnelController)_controllers.get(i);
+                TunnelController controller = _controllers.get(i);
                 if (controller.getStartOnLoad())
                     controller.startTunnel();
             }
@@ -141,10 +142,10 @@ public class TunnelControllerGroup {
      *
      * @return list of messages from the controller as it is stopped
      */
-    public List removeController(TunnelController controller) {
+    public List<String> removeController(TunnelController controller) {
         if (controller == null) return new ArrayList();
         controller.stopTunnel();
-        List msgs = controller.clearMessages();
+        List<String> msgs = controller.clearMessages();
         _controllers.remove(controller);
         msgs.add("Tunnel " + controller.getName() + " removed");
         return msgs;
@@ -155,10 +156,10 @@ public class TunnelControllerGroup {
      *
      * @return list of messages the tunnels generate when stopped
      */
-    public List stopAllControllers() {
+    public List<String> stopAllControllers() {
-        List msgs = new ArrayList();
+        List<String> msgs = new ArrayList();
         for (int i = 0; i < _controllers.size(); i++) {
-            TunnelController controller = (TunnelController)_controllers.get(i);
+            TunnelController controller = _controllers.get(i);
             controller.stopTunnel();
             msgs.addAll(controller.clearMessages());
         }
@@ -172,10 +173,10 @@ public class TunnelControllerGroup {
      *
      * @return list of messages the tunnels generate when started
      */
-    public List startAllControllers() {
+    public List<String> startAllControllers() {
-        List msgs = new ArrayList();
+        List<String> msgs = new ArrayList();
         for (int i = 0; i < _controllers.size(); i++) {
-            TunnelController controller = (TunnelController)_controllers.get(i);
+            TunnelController controller = _controllers.get(i);
             controller.startTunnelBackground();
             msgs.addAll(controller.clearMessages());
         }
@@ -190,10 +191,10 @@ public class TunnelControllerGroup {
      *
      * @return list of messages the tunnels generate when restarted
      */
-    public List restartAllControllers() {
+    public List<String> restartAllControllers() {
-        List msgs = new ArrayList();
+        List<String> msgs = new ArrayList();
         for (int i = 0; i < _controllers.size(); i++) {
-            TunnelController controller = (TunnelController)_controllers.get(i);
+            TunnelController controller = _controllers.get(i);
             controller.restartTunnel();
             msgs.addAll(controller.clearMessages());
         }
@@ -207,10 +208,10 @@ public class TunnelControllerGroup {
      *
      * @return list of messages the tunnels have generated
      */
-    public List clearAllMessages() {
+    public List<String> clearAllMessages() {
-        List msgs = new ArrayList();
+        List<String> msgs = new ArrayList();
         for (int i = 0; i < _controllers.size(); i++) {
-            TunnelController controller = (TunnelController)_controllers.get(i);
+            TunnelController controller = _controllers.get(i);
             msgs.addAll(controller.clearMessages());
         }
         return msgs;
@@ -240,7 +241,7 @@ public class TunnelControllerGroup {
         
         TreeMap map = new TreeMap();
         for (int i = 0; i < _controllers.size(); i++) {
-            TunnelController controller = (TunnelController)_controllers.get(i);
+            TunnelController controller = _controllers.get(i);
             Properties cur = controller.getConfig("tunnel." + i + ".");
             map.putAll(cur);
         }
@@ -296,7 +297,7 @@ public class TunnelControllerGroup {
      *
      * @return list of TunnelController objects
      */
-    public List getControllers() { return _controllers; }
+    public List<TunnelController> getControllers() { return _controllers; }
     
     
     /** 
@@ -306,7 +307,7 @@ public class TunnelControllerGroup {
      */
     void acquire(TunnelController controller, I2PSession session) {
         synchronized (_sessions) {
-            Set owners = (Set)_sessions.get(session);
+            Set<TunnelController> owners = _sessions.get(session);
             if (owners == null) {
                 owners = new HashSet(1);
                 _sessions.put(session, owners);
@@ -326,7 +327,7 @@ public class TunnelControllerGroup {
     void release(TunnelController controller, I2PSession session) {
         boolean shouldClose = false;
         synchronized (_sessions) {
-            Set owners = (Set)_sessions.get(session);
+            Set<TunnelController> owners = _sessions.get(session);
             if (owners != null) {
                 owners.remove(controller);
                 if (owners.isEmpty()) {

history.txt

@@ -1,3 +1,9 @@
+2010-07-01 zzz
+    * EventDispatcher: Minor cleanups and comments
+    * I2PTunnel: Don't start a tunnel if no valid destinations;
+      cleanups, logging, and error propagation fixes
+    * Transport: Fix NTCP address generation when host is specified but port is auto
+
 2010-06-29 sponge
     * 25%-50% cpu savings in BOB. The remainder of the fix is in streaming
       lib, which aparently keeps running and does not sleep according to

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 = 5;
+    public final static long BUILD = 6;
 
     /** for example "-test" */
     public final static String EXTRA = "";