From 9ad8f35bcaec1b00a75f192191eef313e7f164ea Mon Sep 17 00:00:00 2001 From: zzz <zzz@mail.i2p> Date: Fri, 17 Jun 2011 18:37:02 +0000 Subject: [PATCH] * Shutdown: - Kill the global app context - Recognize multi-router case - Fix RandomIterator, YKGenerator, DHBuilder, NTCPConnection hanging on to old context - probably other offenders not yet found - Fix DHBuilder thread not stopping --- core/java/src/net/i2p/I2PAppContext.java | 12 +++++-- .../net/i2p/crypto/DHSessionKeyBuilder.java | 25 +++++++------- core/java/src/net/i2p/crypto/YKGenerator.java | 22 +++++++------ .../net/i2p/data/i2np/I2NPMessageHandler.java | 4 +-- .../net/i2p/data/i2np/I2NPMessageImpl.java | 4 +-- router/java/src/net/i2p/router/Router.java | 24 +++++++++----- .../src/net/i2p/router/RouterContext.java | 33 +++++++++++++++++-- .../router/transport/ntcp/NTCPConnection.java | 13 ++++++++ .../router/transport/ntcp/NTCPTransport.java | 1 + .../net/i2p/router/util/RandomIterator.java | 2 +- 10 files changed, 101 insertions(+), 39 deletions(-) diff --git a/core/java/src/net/i2p/I2PAppContext.java b/core/java/src/net/i2p/I2PAppContext.java index be35d32f13..146d4e0e41 100644 --- a/core/java/src/net/i2p/I2PAppContext.java +++ b/core/java/src/net/i2p/I2PAppContext.java @@ -115,6 +115,10 @@ public class I2PAppContext { * Pull the default context, creating a new one if necessary, else using * the first one created. * + * Warning - do not save the returned value, or the value of any methods below, + * in a static field, or you will get the old context if a new router is + * started in the same JVM after the first is shut down, + * e.g. on Android. */ public static I2PAppContext getGlobalContext() { // skip the global lock - _gAC must be volatile @@ -165,8 +169,12 @@ public class I2PAppContext { private I2PAppContext(boolean doInit, Properties envProps) { if (doInit) { synchronized (I2PAppContext.class) { - if (_globalAppContext == null) + if (_globalAppContext == null) { _globalAppContext = this; + } else { + System.out.println("Warning - New context not replacing old one, you now have a second one"); + (new Exception("I did it")).printStackTrace(); + } } } _overrideProps = new I2PProperties(); @@ -186,7 +194,7 @@ public class I2PAppContext { _elGamalAESEngineInitialized = false; _logManagerInitialized = false; _keyRingInitialized = false; - _shutdownTasks = new ConcurrentHashSet(16); + _shutdownTasks = new ConcurrentHashSet(32); initializeDirs(); } diff --git a/core/java/src/net/i2p/crypto/DHSessionKeyBuilder.java b/core/java/src/net/i2p/crypto/DHSessionKeyBuilder.java index fb3f6eefcf..072fa50b91 100644 --- a/core/java/src/net/i2p/crypto/DHSessionKeyBuilder.java +++ b/core/java/src/net/i2p/crypto/DHSessionKeyBuilder.java @@ -47,8 +47,8 @@ import net.i2p.util.RandomSource; * @author jrandom */ public class DHSessionKeyBuilder { - private static final I2PAppContext _context = I2PAppContext.getGlobalContext(); - private static final Log _log; + private static I2PAppContext _context = I2PAppContext.getGlobalContext(); + private static Log _log; private static final int MIN_NUM_BUILDERS; private static final int MAX_NUM_BUILDERS; private static final int CALC_DELAY; @@ -100,15 +100,18 @@ public class DHSessionKeyBuilder { startPrecalc(); } - /** @since 0.8.8 */ + /** + * Caller must synch on class + * @since 0.8.8 + */ private static void startPrecalc() { - synchronized(DHSessionKeyBuilder.class) { - _precalcThread = new I2PThread(new DHSessionKeyBuilderPrecalcRunner(MIN_NUM_BUILDERS, MAX_NUM_BUILDERS), - "DH Precalc", true); - _precalcThread.setPriority(Thread.MIN_PRIORITY); - _isRunning = true; - _precalcThread.start(); - } + _context = I2PAppContext.getGlobalContext(); + _log = _context.logManager().getLog(DHSessionKeyBuilder.class); + _precalcThread = new I2PThread(new DHSessionKeyBuilderPrecalcRunner(MIN_NUM_BUILDERS, MAX_NUM_BUILDERS), + "DH Precalc", true); + _precalcThread.setPriority(Thread.MIN_PRIORITY); + _isRunning = true; + _precalcThread.start(); } /** @@ -505,7 +508,7 @@ public class DHSessionKeyBuilder { } public void run() { - while (true) { + while (_isRunning) { int curSize = 0; long start = System.currentTimeMillis(); diff --git a/core/java/src/net/i2p/crypto/YKGenerator.java b/core/java/src/net/i2p/crypto/YKGenerator.java index 168ce1f5b2..9a261a5db8 100644 --- a/core/java/src/net/i2p/crypto/YKGenerator.java +++ b/core/java/src/net/i2p/crypto/YKGenerator.java @@ -43,7 +43,7 @@ class YKGenerator { private static final int CALC_DELAY; private static final LinkedBlockingQueue<BigInteger[]> _values; private static Thread _precalcThread; - private static final I2PAppContext ctx; + private static I2PAppContext ctx; private static volatile boolean _isRunning; public final static String PROP_YK_PRECALC_MIN = "crypto.yk.precalc.min"; @@ -76,20 +76,22 @@ class YKGenerator { // _log.debug("ElGamal YK Precalc (minimum: " + MIN_NUM_BUILDERS + " max: " + MAX_NUM_BUILDERS + ", delay: " // + CALC_DELAY + ")"); - ctx.statManager().createRateStat("crypto.YKUsed", "Need a YK from the queue", "Encryption", new long[] { 60*60*1000 }); - ctx.statManager().createRateStat("crypto.YKEmpty", "YK queue empty", "Encryption", new long[] { 60*60*1000 }); startPrecalc(); } - /** @since 0.8.8 */ + /** + * Caller must synch on class + * @since 0.8.8 + */ private static void startPrecalc() { - synchronized(YKGenerator.class) { - _precalcThread = new I2PThread(new YKPrecalcRunner(MIN_NUM_BUILDERS, MAX_NUM_BUILDERS), + ctx = I2PAppContext.getGlobalContext(); + ctx.statManager().createRateStat("crypto.YKUsed", "Need a YK from the queue", "Encryption", new long[] { 60*60*1000 }); + ctx.statManager().createRateStat("crypto.YKEmpty", "YK queue empty", "Encryption", new long[] { 60*60*1000 }); + _precalcThread = new I2PThread(new YKPrecalcRunner(MIN_NUM_BUILDERS, MAX_NUM_BUILDERS), "YK Precalc", true); - _precalcThread.setPriority(Thread.MIN_PRIORITY); - _isRunning = true; - _precalcThread.start(); - } + _precalcThread.setPriority(Thread.MIN_PRIORITY); + _isRunning = true; + _precalcThread.start(); } /** diff --git a/router/java/src/net/i2p/data/i2np/I2NPMessageHandler.java b/router/java/src/net/i2p/data/i2np/I2NPMessageHandler.java index 00493c9b47..a230eb929a 100644 --- a/router/java/src/net/i2p/data/i2np/I2NPMessageHandler.java +++ b/router/java/src/net/i2p/data/i2np/I2NPMessageHandler.java @@ -22,8 +22,8 @@ import net.i2p.util.Log; * */ public class I2NPMessageHandler { - private Log _log; - private I2PAppContext _context; + private final Log _log; + private final I2PAppContext _context; private long _lastReadBegin; private long _lastReadEnd; private int _lastSize; diff --git a/router/java/src/net/i2p/data/i2np/I2NPMessageImpl.java b/router/java/src/net/i2p/data/i2np/I2NPMessageImpl.java index eebb05f7e4..19990a7c84 100644 --- a/router/java/src/net/i2p/data/i2np/I2NPMessageImpl.java +++ b/router/java/src/net/i2p/data/i2np/I2NPMessageImpl.java @@ -28,8 +28,8 @@ import net.i2p.util.SimpleByteCache; * @author jrandom */ public abstract class I2NPMessageImpl extends DataStructureImpl implements I2NPMessage { - private Log _log; - protected I2PAppContext _context; + private final Log _log; + protected final I2PAppContext _context; private long _expiration; private long _uniqueId; diff --git a/router/java/src/net/i2p/router/Router.java b/router/java/src/net/i2p/router/Router.java index b86e8ec22e..c9725fc0a0 100644 --- a/router/java/src/net/i2p/router/Router.java +++ b/router/java/src/net/i2p/router/Router.java @@ -980,19 +980,23 @@ public class Router { try { _context.inNetMessagePool().shutdown(); } catch (Throwable t) { _log.log(Log.CRIT, "Error shutting down the inbound net pool", t); } //try { _sessionKeyPersistenceHelper.shutdown(); } catch (Throwable t) { _log.log(Log.CRIT, "Error shutting down the session key manager", t); } _context.deleteTempDir(); - RouterContext.listContexts().remove(_context); + List<RouterContext> contexts = RouterContext.getContexts(); + contexts.remove(_context); // shut down I2PAppContext tasks here - // TODO if there are multiple routers in the JVM, we don't want to do this + // If there are multiple routers in the JVM, we don't want to do this // to the DH or YK tasks, as they are singletons. - // Have MultiRouter set a property or something. - try { - DHSessionKeyBuilder.shutdown(); - } catch (Throwable t) { _log.log(Log.CRIT, "Error shutting DH", t); } - try { - _context.elGamalEngine().shutdown(); - } catch (Throwable t) { _log.log(Log.CRIT, "Error shutting elGamal", t); } + if (contexts.isEmpty()) { + try { + DHSessionKeyBuilder.shutdown(); + } catch (Throwable t) { _log.log(Log.CRIT, "Error shutting DH", t); } + try { + _context.elGamalEngine().shutdown(); + } catch (Throwable t) { _log.log(Log.CRIT, "Error shutting elGamal", t); } + } else { + _log.logAlways(Log.WARN, "Warning - " + contexts.size() + " routers remaining in this JVM, not releasing all resources"); + } try { ((FortunaRandomSource)_context.random()).shutdown(); } catch (Throwable t) { _log.log(Log.CRIT, "Error shutting random()", t); } @@ -1020,6 +1024,8 @@ public class Router { File f = getPingFile(); f.delete(); + if (RouterContext.getContexts().isEmpty()) + RouterContext.killGlobalContext(); if (_killVMOnEnd) { try { Thread.sleep(1000); } catch (InterruptedException ie) {} Runtime.getRuntime().halt(exitCode); diff --git a/router/java/src/net/i2p/router/RouterContext.java b/router/java/src/net/i2p/router/RouterContext.java index 26e54ff4d9..985f6df4bd 100644 --- a/router/java/src/net/i2p/router/RouterContext.java +++ b/router/java/src/net/i2p/router/RouterContext.java @@ -1,6 +1,7 @@ package net.i2p.router; import java.util.ArrayList; +import java.util.Collections; import java.util.List; import java.util.Properties; @@ -67,6 +68,8 @@ public class RouterContext extends I2PAppContext { // to init everything. Caller MUST call initAll() afterwards. // Sorry, this breaks some main() unit tests out there. //initAll(); + if (!_contexts.isEmpty()) + System.out.println("Warning - More than one router in this JVM"); _contexts.add(this); } @@ -165,11 +168,37 @@ public class RouterContext extends I2PAppContext { /** * Retrieve the list of router contexts currently instantiated in this JVM. * This will always contain only one item (except when a simulation per the - * MultiRouter is going on), and the list should only be modified when a new + * MultiRouter is going on). + * + * @return an unmodifiable list (as of 0.8.8). May be null or empty. + */ + public static List<RouterContext> listContexts() { + return Collections.unmodifiableList(_contexts); + } + + /** + * Same as listContexts() but package private and modifiable. + * The list should only be modified when a new * context is created or a router is shut down. * + * @since 0.8.8 + */ + static List<RouterContext> getContexts() { + return _contexts; + } + + /** + * Kill the global I2PAppContext, so it isn't still around + * when we restart in the same JVM (Android). + * Only do this if there are no other routers in the JVM. + * + * @since 0.8.8 */ - public static List<RouterContext> listContexts() { return _contexts; } + static void killGlobalContext() { + synchronized (I2PAppContext.class) { + _globalAppContext = null; + } + } /** what router is this context working for? */ public Router router() { return _router; } diff --git a/router/java/src/net/i2p/router/transport/ntcp/NTCPConnection.java b/router/java/src/net/i2p/router/transport/ntcp/NTCPConnection.java index a24b3d9728..36752a0453 100644 --- a/router/java/src/net/i2p/router/transport/ntcp/NTCPConnection.java +++ b/router/java/src/net/i2p/router/transport/ntcp/NTCPConnection.java @@ -1089,6 +1089,10 @@ class NTCPConnection implements FIFOBandwidthLimiter.CompleteListener { } private static final int MAX_HANDLERS = 4; + + /** + * FIXME static queue mixes handlers from different contexts in multirouter JVM + */ private final static LinkedBlockingQueue<I2NPMessageHandler> _i2npHandlers = new LinkedBlockingQueue(MAX_HANDLERS); private final static I2NPMessageHandler acquireHandler(RouterContext ctx) { @@ -1129,6 +1133,15 @@ class NTCPConnection implements FIFOBandwidthLimiter.CompleteListener { _dataReadBufs.offer(buf); } + /** @since 0.8.8 */ + static void releaseResources() { + _i2npHandlers.clear(); + _dataReadBufs.clear(); + synchronized(_bufs) { + _bufs.clear(); + } + } + /** * sizeof(data)+data+pad+crc. * diff --git a/router/java/src/net/i2p/router/transport/ntcp/NTCPTransport.java b/router/java/src/net/i2p/router/transport/ntcp/NTCPTransport.java index 326f659eff..8d943fd071 100644 --- a/router/java/src/net/i2p/router/transport/ntcp/NTCPTransport.java +++ b/router/java/src/net/i2p/router/transport/ntcp/NTCPTransport.java @@ -702,6 +702,7 @@ public class NTCPTransport extends TransportImpl { NTCPConnection con = (NTCPConnection)iter.next(); con.close(); } + NTCPConnection.releaseResources(); // will this work? replaceAddress(null); } diff --git a/router/java/src/net/i2p/router/util/RandomIterator.java b/router/java/src/net/i2p/router/util/RandomIterator.java index db8a560b80..b7da15e9b7 100644 --- a/router/java/src/net/i2p/router/util/RandomIterator.java +++ b/router/java/src/net/i2p/router/util/RandomIterator.java @@ -84,7 +84,7 @@ public class RandomIterator<E> implements Iterator<E> { * <a href="http://www.qbrundage.com/michaelb/pubs/essays/random_number_generation" title="http://www.qbrundage.com/michaelb/pubs/essays/random_number_generation" target="_blank">http://www.qbrundage.com/michaelb/pubs/e…</a> * for some implementations, which are faster than java.util.Random. */ - private static final Random rand = RandomSource.getInstance(); + private final Random rand = RandomSource.getInstance(); /** Used to narrow the range to take random indexes from */ private int lower, upper; -- GitLab