* Shutdown:

- Cancel our JVM shutdown hook when shutting down
    - Run a spinner task so shutdown always completes
    - exit() instead of halt() so other JVM shutdown hooks run
    - Prevent duplicate wrapper notifier hooks
    - Notify the wrapper twice, once for stopping and once for stopped

apps/routerconsole/java/src/net/i2p/router/web/ConfigNetHandler.java

@@ -298,7 +298,7 @@ public class ConfigNetHandler extends FormHandler {
     private void hiddenSwitch() {
         // Full restart required to generate new keys
         // FIXME don't call wrapper if not present, only rekey
-        _context.addShutdownTask(new ConfigServiceHandler.UpdateWrapperManagerAndRekeyTask(Router.EXIT_GRACEFUL_RESTART));
+        ConfigServiceHandler.registerWrapperNotifier(_context, Router.EXIT_GRACEFUL_RESTART, false);
         _context.router().shutdownGracefully(Router.EXIT_GRACEFUL_RESTART);
     }
     

apps/routerconsole/java/src/net/i2p/router/web/ConfigRestartBean.java

@@ -31,7 +31,7 @@ public class ConfigRestartBean {
             // Normal browsers send value, IE sends button label
             if ("shutdownImmediate".equals(action) || _("Shutdown immediately", ctx).equals(action)) {
                 if (ctx.hasWrapper())
-                    ctx.addShutdownTask(new ConfigServiceHandler.UpdateWrapperManagerTask(Router.EXIT_HARD));
+                    ConfigServiceHandler.registerWrapperNotifier(ctx, Router.EXIT_HARD, false);
                 //ctx.router().shutdown(Router.EXIT_HARD); // never returns
                 ctx.router().shutdownGracefully(Router.EXIT_HARD); // give the UI time to respond
             } else if ("cancelShutdown".equals(action) || _("Cancel shutdown", ctx).equals(action) ||
@@ -39,16 +39,16 @@ public class ConfigRestartBean {
                 ctx.router().cancelGracefulShutdown();
             } else if ("restartImmediate".equals(action) || _("Restart immediately", ctx).equals(action)) {
                 if (ctx.hasWrapper())
-                    ctx.addShutdownTask(new ConfigServiceHandler.UpdateWrapperManagerTask(Router.EXIT_HARD_RESTART));
+                    ConfigServiceHandler.registerWrapperNotifier(ctx, Router.EXIT_HARD_RESTART, false);
                 //ctx.router().shutdown(Router.EXIT_HARD_RESTART); // never returns
                 ctx.router().shutdownGracefully(Router.EXIT_HARD_RESTART); // give the UI time to respond
             } else if ("restart".equals(action) || _("Restart", ctx).equals(action)) {
                 if (ctx.hasWrapper())
-                    ctx.addShutdownTask(new ConfigServiceHandler.UpdateWrapperManagerTask(Router.EXIT_GRACEFUL_RESTART));
+                    ConfigServiceHandler.registerWrapperNotifier(ctx, Router.EXIT_GRACEFUL_RESTART, false);
                 ctx.router().shutdownGracefully(Router.EXIT_GRACEFUL_RESTART);
             } else if ("shutdown".equals(action) || _("Shutdown", ctx).equals(action)) {
                 if (ctx.hasWrapper())
-                    ctx.addShutdownTask(new ConfigServiceHandler.UpdateWrapperManagerTask(Router.EXIT_GRACEFUL));
+                    ConfigServiceHandler.registerWrapperNotifier(ctx, Router.EXIT_GRACEFUL, false);
                 ctx.router().shutdownGracefully();
             }
         }

apps/routerconsole/java/src/net/i2p/router/web/ConfigServiceHandler.java

@@ -6,6 +6,7 @@ import java.util.List;
 import net.i2p.apps.systray.SysTray;
 import net.i2p.apps.systray.UrlLauncher;
 import net.i2p.router.Router;
+import net.i2p.router.RouterContext;
 import net.i2p.router.startup.ClientAppConfig;
 
 import org.tanukisoftware.wrapper.WrapperManager;
@@ -17,33 +18,113 @@ import org.tanukisoftware.wrapper.WrapperManager;
  */
 public class ConfigServiceHandler extends FormHandler {
     
-    public static class UpdateWrapperManagerTask implements Runnable {
+    /**
-        private int _exitCode;
+     *  Register two shutdown hooks, one to rekey and/or tell the wrapper we are stopping,
-        public UpdateWrapperManagerTask(int exitCode) {
+     *  and a final one to tell the wrapper we are stopped.
-            _exitCode = exitCode;
+     *
-        }
+     *  @since 0.8.8
-        public void run() {
+     */
-            try {
+    private void registerWrapperNotifier(int code, boolean rekey) {
-                WrapperManager.signalStopped(_exitCode);
+        registerWrapperNotifier(_context, code, rekey);
-            } catch (Throwable t) {
-                t.printStackTrace();
     }
+    
+    /**
+     *  Register two shutdown hooks, one to rekey and/or tell the wrapper we are stopping,
+     *  and a final one to tell the wrapper we are stopped.
+     *
+     *  @since 0.8.8
+     */
+    public static void registerWrapperNotifier(RouterContext ctx, int code, boolean rekey) {
+        Runnable task = new UpdateWrapperOrRekeyTask(rekey, ctx.hasWrapper());
+        ctx.addShutdownTask(task);
+        if (ctx.hasWrapper()) {
+            task = new FinalWrapperTask(code);
+            ctx.addFinalShutdownTask(task);
         }
     }
 
-    public static class UpdateWrapperManagerAndRekeyTask implements Runnable {
+    /**
-        private int _exitCode;
+     *  Rekey and/or tell the wrapper we are stopping,
-        public UpdateWrapperManagerAndRekeyTask(int exitCode) {
+     */
-            _exitCode = exitCode;
+    private static class UpdateWrapperOrRekeyTask implements Runnable {
+        private final boolean _rekey;
+        private final boolean _tellWrapper;
+        private static final int HASHCODE = -123999871;
+        private static final int WAIT = 30*1000;
+
+        public UpdateWrapperOrRekeyTask(boolean rekey, boolean tellWrapper) {
+            _rekey = rekey;
+            _tellWrapper = tellWrapper;
         }
+
         public void run() {
             try {
+                if (_rekey)
                     ContextHelper.getContext(null).router().killKeys();
+                if (_tellWrapper)
+                    WrapperManager.signalStopping(WAIT);
+            } catch (Throwable t) {
+                t.printStackTrace();
+            }
+        }
+
+        /**
+         *  Make them all look the same since the hooks are stored in a set
+         *  and we don't want dups
+         */
+        @Override
+        public int hashCode() {
+            return HASHCODE;
+        }
+
+        /**
+         *  Make them all look the same since the hooks are stored in a set
+         *  and we don't want dups
+         */
+        @Override
+        public boolean equals(Object o) {
+            return (o != null) && (o instanceof UpdateWrapperOrRekeyTask);
+        }
+    }
+
+    /**
+     *  Tell the wrapper we are stopped.
+     *
+     *  @since 0.8.8
+     */
+    private static class FinalWrapperTask implements Runnable {
+        private final int _exitCode;
+        private static final int HASHCODE = 123999871;
+
+        public FinalWrapperTask(int exitCode) {
+            _exitCode = exitCode;
+        }
+
+        public void run() {
+            try {
                 WrapperManager.signalStopped(_exitCode);
             } catch (Throwable t) {
                 t.printStackTrace();
             }
         }
+
+        /**
+         *  Make them all look the same since the hooks are stored in a set
+         *  and we don't want dups
+         */
+        @Override
+        public int hashCode() {
+            return HASHCODE;
+        }
+
+        /**
+         *  Make them all look the same since the hooks are stored in a set
+         *  and we don't want dups
+         */
+        @Override
+        public boolean equals(Object o) {
+            return (o != null) && (o instanceof FinalWrapperTask);
+        }
     }
 
     @Override
@@ -52,12 +133,12 @@ public class ConfigServiceHandler extends FormHandler {
         
         if (_("Shutdown gracefully").equals(_action)) {
             if (_context.hasWrapper())
-                _context.addShutdownTask(new UpdateWrapperManagerTask(Router.EXIT_GRACEFUL));
+                registerWrapperNotifier(Router.EXIT_GRACEFUL, false);
             _context.router().shutdownGracefully();
             addFormNotice(_("Graceful shutdown initiated"));
         } else if (_("Shutdown immediately").equals(_action)) {
             if (_context.hasWrapper())
-                _context.addShutdownTask(new UpdateWrapperManagerTask(Router.EXIT_HARD));
+                registerWrapperNotifier(Router.EXIT_HARD, false);
             _context.router().shutdown(Router.EXIT_HARD);
             addFormNotice(_("Shutdown immediately!  boom bye bye bad bwoy"));
         } else if (_("Cancel graceful shutdown").equals(_action)) {
@@ -66,24 +147,22 @@ public class ConfigServiceHandler extends FormHandler {
         } else if (_("Graceful restart").equals(_action)) {
             // should have wrapper if restart button is visible
             if (_context.hasWrapper())
-                _context.addShutdownTask(new UpdateWrapperManagerTask(Router.EXIT_GRACEFUL_RESTART));
+                registerWrapperNotifier(Router.EXIT_GRACEFUL_RESTART, false);
             _context.router().shutdownGracefully(Router.EXIT_GRACEFUL_RESTART);
             addFormNotice(_("Graceful restart requested"));
         } else if (_("Hard restart").equals(_action)) {
             // should have wrapper if restart button is visible
             if (_context.hasWrapper())
-                _context.addShutdownTask(new UpdateWrapperManagerTask(Router.EXIT_HARD_RESTART));
+                registerWrapperNotifier(Router.EXIT_HARD_RESTART, false);
             _context.router().shutdown(Router.EXIT_HARD_RESTART);
             addFormNotice(_("Hard restart requested"));
         } else if (_("Rekey and Restart").equals(_action)) {
             addFormNotice(_("Rekeying after graceful restart"));
-            // FIXME don't call wrapper if not present, only rekey
+            registerWrapperNotifier(Router.EXIT_GRACEFUL_RESTART, true);
-            _context.addShutdownTask(new UpdateWrapperManagerAndRekeyTask(Router.EXIT_GRACEFUL_RESTART));
             _context.router().shutdownGracefully(Router.EXIT_GRACEFUL_RESTART);
         } else if (_("Rekey and Shutdown").equals(_action)) {
             addFormNotice(_("Rekeying after graceful shutdown"));
-            // FIXME don't call wrapper if not present, only rekey
+            registerWrapperNotifier(Router.EXIT_GRACEFUL, true);
-            _context.addShutdownTask(new UpdateWrapperManagerAndRekeyTask(Router.EXIT_GRACEFUL));
             _context.router().shutdownGracefully(Router.EXIT_GRACEFUL);
         } else if (_("Run I2P on startup").equals(_action)) {
             installService();

apps/routerconsole/java/src/net/i2p/router/web/UpdateHandler.java

@@ -313,7 +313,7 @@ public class UpdateHandler {
     
     protected void restart() {
         if (_context.hasWrapper())
-            _context.addShutdownTask(new ConfigServiceHandler.UpdateWrapperManagerTask(Router.EXIT_GRACEFUL_RESTART));
+            ConfigServiceHandler.registerWrapperNotifier(_context, Router.EXIT_GRACEFUL_RESTART, false);
         _context.router().shutdownGracefully(Router.EXIT_GRACEFUL_RESTART);
     }
 

history.txt

@@ -1,3 +1,11 @@
+2011-07-15 zzz
+  * Shutdown:
+    - Cancel our JVM shutdown hook when shutting down
+    - Run a spinner task so shutdown always completes
+    - exit() instead of halt() so other JVM shutdown hooks run
+    - Prevent duplicate wrapper notifier hooks
+    - Notify the wrapper twice, once for stopping and once for stopped
+
 2011-07-13 zzz
   * Blocklist:
     - Fix delayed lookup of reason from file

router/java/src/net/i2p/router/Router.java

@@ -375,7 +375,9 @@ public class Router implements RouterClock.ClockShiftListener {
     void runRouter() {
         _isAlive = true;
         _started = _context.clock().now();
-        Runtime.getRuntime().addShutdownHook(_shutdownHook);
+        try {
+            Runtime.getRuntime().removeShutdownHook(_shutdownHook);
+        } catch (IllegalStateException ise) {}
         I2PThread.addOOMEventListener(_oomListener);
         
         _context.keyManager().startup();
@@ -644,6 +646,11 @@ public class Router implements RouterClock.ClockShiftListener {
      *
      */
     public void rebuildNewIdentity() {
+        if (_shutdownHook != null) {
+            try {
+                Runtime.getRuntime().removeShutdownHook(_shutdownHook);
+            } catch (IllegalStateException ise) {}
+        }
         killKeys();
         for (Runnable task : _context.getShutdownTasks()) {
             if (_log.shouldLog(Log.WARN))
@@ -961,6 +968,27 @@ public class Router implements RouterClock.ClockShiftListener {
     }
 ******/
 
+    /**
+     *  A non-daemon thread to let
+     *  the shutdown task get all the way to the end
+     *  @since 0.8.8
+     */
+    private static class Spinner extends Thread {
+
+        public Spinner() {
+            super();
+            setName("Shutdown Spinner");
+            setDaemon(false);
+        }
+
+        @Override
+        public void run() {
+            try {
+                sleep(60*1000);
+            } catch (InterruptedException ie) {}
+        }
+    }
+    
     public static final int EXIT_GRACEFUL = 2;
     public static final int EXIT_HARD = 3;
     public static final int EXIT_OOM = 10;
@@ -971,6 +999,25 @@ public class Router implements RouterClock.ClockShiftListener {
      *  Shutdown with no chance of cancellation
      */
     public void shutdown(int exitCode) {
+        if (_shutdownHook != null) {
+            try {
+                Runtime.getRuntime().removeShutdownHook(_shutdownHook);
+            } catch (IllegalStateException ise) {}
+        }
+        shutdown2(exitCode);
+    }
+
+    /**
+     *  Cancel the JVM runtime hook before calling this.
+     */
+    private void shutdown2(int exitCode) {
+        // So we can get all the way to the end
+        // No, you can't do Thread.currentThread.setDaemon(false)
+        if (_killVMOnEnd) {
+            try {
+                (new Spinner()).start();
+            } catch (Throwable t) {}
+        }
         ((RouterClock) _context.clock()).removeShiftListener(this);
         _isAlive = false;
         _context.random().saveSeed();
@@ -1038,6 +1085,9 @@ public class Router implements RouterClock.ClockShiftListener {
      */
     private static final boolean ALLOW_DYNAMIC_KEYS = false;
 
+    /**
+     *  Cancel the JVM runtime hook before calling this.
+     */
     private void finalShutdown(int exitCode) {
         clearCaches();
         _log.log(Log.CRIT, "Shutdown(" + exitCode + ") complete"  /* , new Exception("Shutdown") */ );
@@ -1052,9 +1102,9 @@ public class Router implements RouterClock.ClockShiftListener {
         if (RouterContext.getContexts().isEmpty())
             RouterContext.killGlobalContext();
 
-        // Since 0.8.8, mainly for Android
+        // Since 0.8.8, for Android and the wrapper
         for (Runnable task : _context.getFinalShutdownTasks()) {
-            System.err.println("Running final shutdown task " + task.getClass());
+            //System.err.println("Running final shutdown task " + task.getClass());
             try {
                 task.run();
             } catch (Throwable t) {
@@ -1065,7 +1115,9 @@ public class Router implements RouterClock.ClockShiftListener {
 
         if (_killVMOnEnd) {
             try { Thread.sleep(1000); } catch (InterruptedException ie) {}
-            Runtime.getRuntime().halt(exitCode);
+            //Runtime.getRuntime().halt(exitCode);
+            // allow the Runtime shutdown hooks to execute
+            Runtime.getRuntime().exit(exitCode);
         } else {
             Runtime.getRuntime().gc();
         }
@@ -1772,20 +1824,25 @@ private static class MarkLiveliness implements SimpleTimer.TimedEvent {
     }
 }
 
+/**
+ *  Just for failsafe. Standard shutdown should cancel this.
+ */
 private static class ShutdownHook extends Thread {
-    private RouterContext _context;
+    private final RouterContext _context;
     private static int __id = 0;
-    private int _id;
+    private final int _id;
+
     public ShutdownHook(RouterContext ctx) {
         _context = ctx;
         _id = ++__id;
     }
+
     @Override
     public void run() {
         setName("Router " + _id + " shutdown");
         Log l = _context.logManager().getLog(Router.class);
         l.log(Log.CRIT, "Shutting down the router...");
-        _context.router().shutdown(Router.EXIT_HARD);
+        _context.router().shutdown2(Router.EXIT_HARD);
     }
 }
 

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