diff --git a/router/java/src/net/i2p/router/transport/UPnP.java b/router/java/src/net/i2p/router/transport/UPnP.java index c20fc2548..b8572f726 100644 --- a/router/java/src/net/i2p/router/transport/UPnP.java +++ b/router/java/src/net/i2p/router/transport/UPnP.java @@ -263,9 +263,10 @@ public class UPnP extends ControlPoint implements DeviceChangeListener, EventLis } } - /** event callback */ + /** event callback - unused for now - how many devices support events? */ public void eventNotifyReceived(String uuid, long seq, String varName, String value) { - _log.error("Event: " + uuid + ' ' + seq + ' ' + varName + '=' + value); + if (_log.shouldLog(Log.WARN)) + _log.error("Event: " + uuid + ' ' + seq + ' ' + varName + '=' + value); } /** compare two strings, either of which could be null */ @@ -304,7 +305,7 @@ public class UPnP extends ControlPoint implements DeviceChangeListener, EventLis /** * @return the reported upstream bit rate in bits per second. -1 if it's not available. Blocking. */ - public int getUpstramMaxBitRate() { + public int getUpstreamMaxBitRate() { if(!isNATPresent() || thinksWeAreDoubleNatted) return -1; @@ -361,13 +362,32 @@ public class UPnP extends ControlPoint implements DeviceChangeListener, EventLis } ***/ - private String toString(String action, String Argument, Service serv) { - Action getIP = serv.getAction(action); - if(getIP == null || !getIP.postControlAction()) - return null; - - Argument ret = getIP.getOutputArgumentList().getArgument(Argument); - return ret.getValue(); + /** + * A blocking toString(). That's interesting. + * Cache the last ArgumentList to speed it up some. + * Count on listSubServices() to call multiple combinations of arguments + * so we don't get old data. + */ + private String _lastAction; + private Service _lastService; + private ArgumentList _lastArgumentList; + private Object toStringLock = new Object(); + private String toString(String action, String arg, Service serv) { + synchronized(toStringLock) { + if ((!action.equals(_lastAction)) || + (!serv.equals(_lastService)) || + _lastArgumentList == null) { + Action getIP = serv.getAction(action); + if(getIP == null || !getIP.postControlAction()) { + _lastAction = null; + return null; + } + _lastAction = action; + _lastService = serv; + _lastArgumentList = getIP.getOutputArgumentList(); + } + return _lastArgumentList.getArgument(arg).getValue(); + } } // TODO: extend it! RTFM @@ -433,6 +453,7 @@ public class UPnP extends ControlPoint implements DeviceChangeListener, EventLis sb.append("\n"); } + /** warning - slow */ public String renderStatusHTML() { final StringBuilder sb = new StringBuilder(); sb.append("UPnP Status:
"); @@ -454,11 +475,11 @@ public class UPnP extends ControlPoint implements DeviceChangeListener, EventLis else sb.append("
The current external IP address is not available."); int downstreamMaxBitRate = getDownstreamMaxBitRate(); - int upstreamMaxBitRate = getUpstramMaxBitRate(); + int upstreamMaxBitRate = getUpstreamMaxBitRate(); if(downstreamMaxBitRate > 0) - sb.append("
UPnP reports the max downstream bit rate is : " + getDownstreamMaxBitRate()+ " bits/sec\n"); + sb.append("
UPnP reports the max downstream bit rate is : " + downstreamMaxBitRate+ " bits/sec\n"); if(upstreamMaxBitRate > 0) - sb.append("
UPnP reports the max upstream bit rate is : " + getUpstramMaxBitRate()+ " bits/sec\n"); + sb.append("
UPnP reports the max upstream bit rate is : " + upstreamMaxBitRate+ " bits/sec\n"); synchronized(lock) { if(portsToForward != null) { for(ForwardPort port : portsToForward) { @@ -475,6 +496,7 @@ public class UPnP extends ControlPoint implements DeviceChangeListener, EventLis return sb.toString(); } + /** blocking */ private boolean addMapping(String protocol, int port, String description, ForwardPort fp) { if(isDisabled || !isNATPresent() || _router == null) { _log.error("Can't addMapping: " + isDisabled + " " + isNATPresent() + " " + _router); @@ -509,6 +531,7 @@ public class UPnP extends ControlPoint implements DeviceChangeListener, EventLis } else return false; } + /** blocking */ private boolean removeMapping(String protocol, int port, ForwardPort fp, boolean noLog) { if(isDisabled || !isNATPresent()) return false; @@ -533,6 +556,7 @@ public class UPnP extends ControlPoint implements DeviceChangeListener, EventLis return retval; } + /** non-blocking */ public void onChangePublicPorts(Set ports, ForwardPortCallback cb) { Set portsToDumpNow = null; Set portsToForwardNow = null; @@ -555,7 +579,9 @@ public class UPnP extends ControlPoint implements DeviceChangeListener, EventLis // Ports in ports but not in portsToForwardNow we must forward // Ports in portsToForwardNow but not in ports we must dump for(ForwardPort port: ports) { - if(portsToForward.contains(port)) { + //if(portsToForward.contains(port)) { + // If not in portsForwarded, it wasn't successful, try again + if(portsForwarded.contains(port)) { // We have forwarded it, and it should be forwarded, cool. } else { // Needs forwarding diff --git a/router/java/src/net/i2p/router/transport/UPnPManager.java b/router/java/src/net/i2p/router/transport/UPnPManager.java index 6201d56d9..a50f8488a 100644 --- a/router/java/src/net/i2p/router/transport/UPnPManager.java +++ b/router/java/src/net/i2p/router/transport/UPnPManager.java @@ -76,7 +76,12 @@ public class UPnPManager { _detectedAddress = null; } - /** call when the ports might have changed */ + /** + * Call when the ports might have changed + * The transports can call this pretty quickly at startup, + * which can have multiple UPnP threads running at once, but + * that should be ok. + */ public void update(Map ports) { if (_log.shouldLog(Log.DEBUG)) _log.debug("UPnP Update:"); @@ -97,6 +102,7 @@ public class UPnPManager { ForwardPort fp = new ForwardPort(style, false, protocol, port); forwards.add(fp); } + // non-blocking _upnp.onChangePublicPorts(forwards, _upnpCallback); } diff --git a/router/java/src/org/cybergarage/http/HTTPRequest.java b/router/java/src/org/cybergarage/http/HTTPRequest.java index f8b79087c..9e03adc89 100644 --- a/router/java/src/org/cybergarage/http/HTTPRequest.java +++ b/router/java/src/org/cybergarage/http/HTTPRequest.java @@ -386,13 +386,14 @@ public class HTTPRequest extends HTTPPacket // as when the device goes away, this hangs the display of peers.jsp // and who knows what else. // Set the timeout to be nice and short, the device should be local and fast. - // If he can't get back to us in 2 seconds, forget it. + // Yeah, the UPnP standard is a minute or something, too bad. + // If he can't get back to us in a few seconds, forget it. // And set the soTimeout to 1 second (for reads). //postSocket = new Socket(host, port); postSocket = new Socket(); postSocket.setSoTimeout(1000); SocketAddress sa = new InetSocketAddress(host, port); - postSocket.connect(sa, 2000); + postSocket.connect(sa, 3000); } out = postSocket.getOutputStream();