From 0bf0715adc579c59fe624daf86005dbc321457af Mon Sep 17 00:00:00 2001
From: zzz <zzz@mail.i2p>
Date: Fri, 15 Jul 2011 01:13:35 +0000
Subject: [PATCH]   * 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

---
 .../net/i2p/router/web/ConfigNetHandler.java  |   2 +-
 .../net/i2p/router/web/ConfigRestartBean.java |   8 +-
 .../i2p/router/web/ConfigServiceHandler.java  | 115 +++++++++++++++---
 .../src/net/i2p/router/web/UpdateHandler.java |   2 +-
 history.txt                                   |   8 ++
 router/java/src/net/i2p/router/Router.java    |  73 +++++++++--
 .../src/net/i2p/router/RouterVersion.java     |   2 +-
 7 files changed, 177 insertions(+), 33 deletions(-)

diff --git a/apps/routerconsole/java/src/net/i2p/router/web/ConfigNetHandler.java b/apps/routerconsole/java/src/net/i2p/router/web/ConfigNetHandler.java
index 4d2b73bc32..d75bbd5cd8 100644
--- a/apps/routerconsole/java/src/net/i2p/router/web/ConfigNetHandler.java
+++ b/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);
     }
     
diff --git a/apps/routerconsole/java/src/net/i2p/router/web/ConfigRestartBean.java b/apps/routerconsole/java/src/net/i2p/router/web/ConfigRestartBean.java
index e98dadcac3..8839b82feb 100644
--- a/apps/routerconsole/java/src/net/i2p/router/web/ConfigRestartBean.java
+++ b/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();
             }
         }
diff --git a/apps/routerconsole/java/src/net/i2p/router/web/ConfigServiceHandler.java b/apps/routerconsole/java/src/net/i2p/router/web/ConfigServiceHandler.java
index 304b624a03..170af5c355 100644
--- a/apps/routerconsole/java/src/net/i2p/router/web/ConfigServiceHandler.java
+++ b/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,47 +18,127 @@ import org.tanukisoftware.wrapper.WrapperManager;
  */
 public class ConfigServiceHandler extends FormHandler {
     
-    public static class UpdateWrapperManagerTask implements Runnable {
-        private int _exitCode;
-        public UpdateWrapperManagerTask(int exitCode) {
-            _exitCode = exitCode;
+    /**
+     *  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
+     */
+    private void registerWrapperNotifier(int code, boolean rekey) {
+        registerWrapperNotifier(_context, code, rekey);
+    }
+    
+    /**
+     *  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);
         }
+    }
+
+    /**
+     *  Rekey and/or tell the wrapper we are stopping,
+     */
+    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 {
-                WrapperManager.signalStopped(_exitCode);
+                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);
+        }
     }
 
-    public static class UpdateWrapperManagerAndRekeyTask implements Runnable {
-        private int _exitCode;
-        public UpdateWrapperManagerAndRekeyTask(int exitCode) {
+    /**
+     *  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 {
-                ContextHelper.getContext(null).router().killKeys();
                 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
     protected void processForm() {
         if (_action == null) return;
         
         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
-            _context.addShutdownTask(new UpdateWrapperManagerAndRekeyTask(Router.EXIT_GRACEFUL_RESTART));
+            registerWrapperNotifier(Router.EXIT_GRACEFUL_RESTART, true);
             _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
-            _context.addShutdownTask(new UpdateWrapperManagerAndRekeyTask(Router.EXIT_GRACEFUL));
+            registerWrapperNotifier(Router.EXIT_GRACEFUL, true);
             _context.router().shutdownGracefully(Router.EXIT_GRACEFUL);
         } else if (_("Run I2P on startup").equals(_action)) {
             installService();
diff --git a/apps/routerconsole/java/src/net/i2p/router/web/UpdateHandler.java b/apps/routerconsole/java/src/net/i2p/router/web/UpdateHandler.java
index 4581a346a7..2f2d3db74f 100644
--- a/apps/routerconsole/java/src/net/i2p/router/web/UpdateHandler.java
+++ b/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);
     }
 
diff --git a/history.txt b/history.txt
index 57bbb375fb..1461bd8a12 100644
--- a/history.txt
+++ b/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
diff --git a/router/java/src/net/i2p/router/Router.java b/router/java/src/net/i2p/router/Router.java
index 23990673c7..e513234253 100644
--- a/router/java/src/net/i2p/router/Router.java
+++ b/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))
@@ -960,6 +967,27 @@ public class Router implements RouterClock.ClockShiftListener {
         buf.setLength(0);
     }
 ******/
+
+    /**
+     *  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;
@@ -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
+
+    @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);
     }
 }
 
diff --git a/router/java/src/net/i2p/router/RouterVersion.java b/router/java/src/net/i2p/router/RouterVersion.java
index 7ca1a3ec2c..5a58b19bc2 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 = 10;
+    public final static long BUILD = 11;
 
     /** for example "-test" */
     public final static String EXTRA = "";
-- 
GitLab