diff --git a/history.txt b/history.txt index 229e72fdcc..7ec5b7bcc9 100644 --- a/history.txt +++ b/history.txt @@ -1,3 +1,7 @@ +2015-10-31 zzz + * Convert remaining Threads to I2PThread or I2PAppThread + * UPnP: Fix deadlock in callbacks (ticket #1699) + 2015-10-30 zzz * Router: Fix cascading I2CP error (ticket #1692) diff --git a/router/java/src/net/i2p/router/RouterVersion.java b/router/java/src/net/i2p/router/RouterVersion.java index f5c9264549..43dca92409 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 = 20; + public final static long BUILD = 21; /** for example "-test" */ public final static String EXTRA = ""; diff --git a/router/java/src/net/i2p/router/transport/UPnPManager.java b/router/java/src/net/i2p/router/transport/UPnPManager.java index 370ba813e8..9d7a6918a0 100644 --- a/router/java/src/net/i2p/router/transport/UPnPManager.java +++ b/router/java/src/net/i2p/router/transport/UPnPManager.java @@ -233,7 +233,17 @@ class UPnPManager { public void portForwardStatus(Map statuses) { if (_log.shouldLog(Log.DEBUG)) _log.debug("UPnP Callback:"); + // Let's not have two of these running at once. + // Deadlock reported in ticket #1699 + // and the locking isn't foolproof in UDPTransport. + // UPnP runs the callbacks in a thread, so we can block. + // There is only one UPnPCallback, so lock on this + synchronized(this) { + locked_PFS(statuses); + } + } + private void locked_PFS(Map statuses) { byte[] ipaddr = null; DetectedIP[] ips = _upnp.getAddress(); if (ips != null) { @@ -244,6 +254,7 @@ class UPnPManager { _log.debug("External address: " + ip.publicAddress + " type: " + ip.natType); if (!ip.publicAddress.equals(_detectedAddress)) { _detectedAddress = ip.publicAddress; + // deadlock path 1 _manager.externalAddressReceived(SOURCE_UPNP, _detectedAddress.getAddress(), 0); } ipaddr = ip.publicAddress.getAddress(); @@ -269,6 +280,7 @@ class UPnPManager { else continue; boolean success = fps.status >= ForwardPortStatus.MAYBE_SUCCESS; + // deadlock path 2 _manager.forwardPortStatus(style, ipaddr, fp.portNumber, fps.externalPort, success, fps.reasonString); } } diff --git a/router/java/src/net/i2p/router/transport/udp/UDPTransport.java b/router/java/src/net/i2p/router/transport/udp/UDPTransport.java index 223f655f5a..edcfef26bb 100644 --- a/router/java/src/net/i2p/router/transport/udp/UDPTransport.java +++ b/router/java/src/net/i2p/router/transport/udp/UDPTransport.java @@ -993,6 +993,7 @@ public class UDPTransport extends TransportImpl implements TimedWeightedPriority // save PROP_EXTERNAL_PORT _context.router().saveConfig(changes, null); } + // deadlock thru here ticket #1699 _context.router().rebuildRouterInfo(); } _testEvent.forceRunImmediately(); @@ -2115,6 +2116,7 @@ public class UDPTransport extends TransportImpl implements TimedWeightedPriority * @since 0.9.18 */ private RouterAddress getCurrentExternalAddress(boolean isIPv6) { + // deadlock thru here ticket #1699 synchronized (_rebuildLock) { return isIPv6 ? _currentOurV6Address : _currentOurV4Address; }