diff --git a/apps/i2ptunnel/java/src/net/i2p/i2ptunnel/I2PTunnel.java b/apps/i2ptunnel/java/src/net/i2p/i2ptunnel/I2PTunnel.java index 363cd0679..3a7e986d8 100644 --- a/apps/i2ptunnel/java/src/net/i2p/i2ptunnel/I2PTunnel.java +++ b/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 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.

* + * 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 [,]|file:[ ] []"); @@ -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 [] []"); @@ -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 [] []"); @@ -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 [ []]"); diff --git a/apps/i2ptunnel/java/src/net/i2p/i2ptunnel/I2PTunnelClient.java b/apps/i2ptunnel/java/src/net/i2p/i2ptunnel/I2PTunnelClient.java index 89339bf04..d5b88d4d3 100644 --- a/apps/i2ptunnel/java/src/net/i2p/i2ptunnel/I2PTunnelClient.java +++ b/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 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); } } diff --git a/apps/i2ptunnel/java/src/net/i2p/i2ptunnel/I2PTunnelClientBase.java b/apps/i2ptunnel/java/src/net/i2p/i2ptunnel/I2PTunnelClientBase.java index 398fd7349..71a1561ea 100644 --- a/apps/i2ptunnel/java/src/net/i2p/i2ptunnel/I2PTunnelClientBase.java +++ b/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 diff --git a/apps/i2ptunnel/java/src/net/i2p/i2ptunnel/I2PTunnelIRCClient.java b/apps/i2ptunnel/java/src/net/i2p/i2ptunnel/I2PTunnelIRCClient.java index a7fbcf37b..2892d71b7 100644 --- a/apps/i2ptunnel/java/src/net/i2p/i2ptunnel/I2PTunnelIRCClient.java +++ b/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 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); } /************************************************************************* diff --git a/apps/i2ptunnel/java/src/net/i2p/i2ptunnel/TunnelController.java b/apps/i2ptunnel/java/src/net/i2p/i2ptunnel/TunnelController.java index 7235507ba..fa0564be8 100644 --- a/apps/i2ptunnel/java/src/net/i2p/i2ptunnel/TunnelController.java +++ b/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 _sessions; + private List _messages; + private List _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(); - if (sessions != null) { + List sessions = _tunnel.getSessions(); + 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()); } } @@ -370,7 +379,7 @@ public class TunnelController implements Logging { _tunnel.port = "7654"; } } - + public void stopTunnel() { _tunnel.runClose(new String[] { "forced", "all" }, this); release(); @@ -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 clearMessages() { List rv = null; synchronized (this) { rv = new ArrayList(_messages); diff --git a/apps/i2ptunnel/java/src/net/i2p/i2ptunnel/TunnelControllerGroup.java b/apps/i2ptunnel/java/src/net/i2p/i2ptunnel/TunnelControllerGroup.java index 112f0d62f..169f56954 100644 --- a/apps/i2ptunnel/java/src/net/i2p/i2ptunnel/TunnelControllerGroup.java +++ b/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 _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> _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 removeController(TunnelController controller) { if (controller == null) return new ArrayList(); controller.stopTunnel(); - List msgs = controller.clearMessages(); + List 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() { - List msgs = new ArrayList(); + public List stopAllControllers() { + List 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() { - List msgs = new ArrayList(); + public List startAllControllers() { + List 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() { - List msgs = new ArrayList(); + public List restartAllControllers() { + List 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() { - List msgs = new ArrayList(); + public List clearAllMessages() { + List 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 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 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 owners = _sessions.get(session); if (owners != null) { owners.remove(controller); if (owners.isEmpty()) { diff --git a/history.txt b/history.txt index 9bd64c102..9f7f8c2dd 100644 --- a/history.txt +++ b/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 diff --git a/router/java/src/net/i2p/router/RouterVersion.java b/router/java/src/net/i2p/router/RouterVersion.java index 4e28a2c8d..c10128fe6 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 = 5; + public final static long BUILD = 6; /** for example "-test" */ public final static String EXTRA = "";