Console: Fix stopping webapps at shutdown (ticket #2508)

RouterAppManager log tweaks

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

@@ -17,6 +17,8 @@ import java.util.Properties;
 import java.util.StringTokenizer;
 import java.util.concurrent.ConcurrentHashMap;
 
+import org.eclipse.jetty.server.Server;
+
 import net.i2p.CoreVersion;
 import net.i2p.I2PAppContext;
 import net.i2p.app.ClientApp;
@@ -541,12 +543,24 @@ public class PluginStarter implements Runnable {
      *  @throws Exception just about anything, caller would be wise to catch Throwable
      */
     public static boolean stopPlugin(RouterContext ctx, String appName) throws Exception {
+        Server s = RouterConsoleRunner.getConsoleServer(ctx);
+        return stopPlugin(ctx, s, appName);
+    }
+
+    /**
+     *  @return true on success
+     *  @throws Exception just about anything, caller would be wise to catch Throwable
+     *  @since 0.9.41
+     */
+    protected static boolean stopPlugin(RouterContext ctx, Server s, String appName) throws Exception {
         Log log = ctx.logManager().getLog(PluginStarter.class);
         File pluginDir = new File(ctx.getConfigDir(), PLUGIN_DIR + '/' + appName);
         if ((!pluginDir.exists()) || (!pluginDir.isDirectory())) {
             log.error("Cannot stop nonexistent plugin: " + appName);
             return false;
         }
+        if (log.shouldLog(Log.WARN))
+            log.warn("Stopping plugin: " + appName);
 
         // stop things in clients.config
         File clientConfig = new File(pluginDir, "clients.config");
@@ -575,13 +589,16 @@ public class PluginStarter implements Runnable {
                 }
             }
         */
-            if(pluginWars.containsKey(appName)) {
+            if (s != null) {
-                Iterator <String> wars = pluginWars.get(appName).iterator();
+                Collection<String> wars = pluginWars.get(appName);
-                while (wars.hasNext()) {
+                if (wars != null) {
-                    String warName = wars.next();
+                    for (String warName : wars) {
-                    WebAppStarter.stopWebApp(ctx, warName);
+                        if (log.shouldInfo())
+                            log.info("Stopping webapp " + warName + " in plugin " + appName);
+                        WebAppStarter.stopWebApp(ctx, s, warName);
+                    }
+                    wars.clear();
                 }
-                pluginWars.get(appName).clear();
             }
         //}
 
@@ -593,8 +610,6 @@ public class PluginStarter implements Runnable {
         if (name != null && name.length() > 0)
             NavHelper.unregisterApp(name);
 
-        if (log.shouldLog(Log.WARN))
-            log.warn("Stopping plugin: " + appName);
         return true;
     }
 
@@ -947,6 +962,14 @@ public class PluginStarter implements Runnable {
     }
 
     public static boolean isPluginRunning(String pluginName, RouterContext ctx) {
+        Server s = RouterConsoleRunner.getConsoleServer(ctx);
+        return isPluginRunning(pluginName, ctx, s);
+    }
+
+    /**
+     *  @since 0.9.41
+     */
+    protected static boolean isPluginRunning(String pluginName, RouterContext ctx, Server s) {
         Log log = ctx.logManager().getLog(PluginStarter.class);
         
         boolean isJobRunning = false;
@@ -955,13 +978,16 @@ public class PluginStarter implements Runnable {
             // TODO have a pending indication too
             isJobRunning = true;
         }
+
         boolean isWarRunning = false;
-        if(pluginWars.containsKey(pluginName)) {
+        if (s != null) {
-            Iterator <String> it = pluginWars.get(pluginName).iterator();
+            Collection<String> wars = pluginWars.get(pluginName);
-            while(it.hasNext() && !isWarRunning) {
+            if (wars != null) {
-                String warName = it.next();
+                for (String warName : wars) {
-                if(WebAppStarter.isWebAppRunning(ctx, warName)) {
+                    if (WebAppStarter.isWebAppRunning(s, warName)) {
                         isWarRunning = true;
+                        break;
+                    }
                 }
             }
         }

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

@@ -2,6 +2,9 @@ package net.i2p.router.web;
 
 import java.util.Collections;
 import java.util.List;
+
+import org.eclipse.jetty.server.Server;
+
 import net.i2p.router.RouterContext;
 import net.i2p.util.Log;
 
@@ -11,15 +14,18 @@ import net.i2p.util.Log;
  *  @since 0.7.13
  *  @author zzz
  */
-public class PluginStopper extends PluginStarter {
+class PluginStopper extends PluginStarter {
 
-    public PluginStopper(RouterContext ctx) {
+    private final Server _server;
+
+    public PluginStopper(RouterContext ctx, Server server) {
         super(ctx);
+        _server = server;
     }
 
     @Override
     public void run() {
-        stopPlugins(_context);
+        stopPlugins();
     }
 
     /**
@@ -27,18 +33,23 @@ public class PluginStopper extends PluginStarter {
      *
      *  this shouldn't throw anything
      */
-    private static void stopPlugins(RouterContext ctx) {
+    private void stopPlugins() {
-        Log log = ctx.logManager().getLog(PluginStopper.class);
+        Log log = _context.logManager().getLog(PluginStopper.class);
         List<String> pl = getPlugins();
         Collections.reverse(pl); // reverse the order
         for (String app : pl) {
-            if (isPluginRunning(app, ctx)) {
+            if (isPluginRunning(app, _context, _server)) {
                 try {
-                   stopPlugin(ctx, app);
+                    if (log.shouldInfo())
+                        log.info("Stopping plugin: " + app);
+                    stopPlugin(_context, _server, app);
                 } catch (Throwable e) {
                    if (log.shouldLog(Log.WARN))
                        log.warn("Failed to stop plugin: " + app, e);
                 }
+            } else {
+                if (log.shouldInfo())
+                    log.info("Plugin not running: " + app);
             }
         }
     }

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

@@ -239,9 +239,10 @@ public class RouterConsoleRunner implements RouterApp {
     public synchronized void shutdown(String[] args) {
         if (_state == STOPPED)
             return;
+        // this unregisters us with the ClientAppManager
         changeState(STOPPING);
         if (PluginStarter.pluginsEnabled(_context))
-            (new I2PAppThread(new PluginStopper(_context), "PluginStopper")).start();
+            (new I2PAppThread(new PluginStopper(_context, _server), "PluginStopper")).start();
         stopAllWebApps();
         try {
             _server.stop();
@@ -293,7 +294,10 @@ public class RouterConsoleRunner implements RouterApp {
     }
 
     /**
-     *  To get to Jetty
+     *  To get to Jetty.
+     *  Warning, this will NOT work during shutdown, because
+     *  changeState(STOPPING) will unregister us first.
+     *
      *  @return may be null or stopped perhaps
      *  @since 0.9.38
      */
@@ -1137,6 +1141,9 @@ public class RouterConsoleRunner implements RouterApp {
      *  @since 0.9.30
      */
     private void stopAllWebApps() {
+        net.i2p.util.Log log = _context.logManager().getLog(RouterConsoleRunner.class);
+        if (log.shouldWarn())
+            log.warn("Stop all webapps");
         Properties props = webAppProperties(_context);
         Set<String> keys = props.stringPropertyNames();
         for (String name : keys) {
@@ -1144,10 +1151,15 @@ public class RouterConsoleRunner implements RouterApp {
                 String app = name.substring(PREFIX.length(), name.lastIndexOf(ENABLED));
                 if (ROUTERCONSOLE.equals(app))
                     continue;
-                if (WebAppStarter.isWebAppRunning(_context, app)) {
+                if (_context.portMapper().isRegistered(app)) {
+                    if (log.shouldWarn())
+                        log.warn("Stopping " + app);
                     try {
-                        WebAppStarter.stopWebApp(_context, app);
+                        WebAppStarter.stopWebApp(_context, _server, app);
                     } catch (Throwable t) { t.printStackTrace(); }
+                } else {
+                    if (log.shouldWarn())
+                        log.info("Not Stoppping, isn't running " + app);
                 }
             }
         }

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

@@ -151,6 +151,10 @@ public class WebAppStarter {
     /**
      *  Stop it and remove the context.
      *  Throws just about anything, caller would be wise to catch Throwable
+     *
+     *  Warning, this will NOT work during shutdown, because
+     *  the console is already unregistered.
+     *
      *  @since public since 0.9.33, was package private
      */
     public static void stopWebApp(RouterContext ctx, String appName) {
@@ -171,10 +175,36 @@ public class WebAppStarter {
         } catch (IllegalStateException ise) {}
     }
 
+    /**
+     *  Stop it and remove the context.
+     *  Throws just about anything, caller would be wise to catch Throwable
+     *  @since 0.9.41
+     */
+    static void stopWebApp(RouterContext ctx, Server s, String appName) {
+        ContextHandlerCollection server = getConsoleServer(s);
+        if (server == null)
+            return;
+        ContextHandler wac = getWebApp(server, appName);
+        if (wac == null)
+            return;
+        ctx.portMapper().unregister(appName);
+        try {
+            // not graceful is default in Jetty 6?
+            wac.stop();
+        } catch (Exception ie) {}
+        try {
+            server.removeHandler(wac);
+            server.mapContexts();
+        } catch (IllegalStateException ise) {}
+    }
+
     /**
      *  As of 0.9.34, the appName will be registered with the PortMapper,
      *  and PortMapper.isRegistered() will be more efficient than this.
      *
+     *  Warning, this will NOT work during shutdown, because
+     *  the console is already unregistered.
+     *
      *  @since public since 0.9.33; was package private
      */
     public static boolean isWebAppRunning(I2PAppContext ctx, String appName) {
@@ -184,11 +214,43 @@ public class WebAppStarter {
         return wac.isStarted();
     }
     
-    /** @since Jetty 6 */
+    /**
+     *  @since 0.9.41
+     */
+    static boolean isWebAppRunning(Server s, String appName) {
+        ContextHandler wac = getWebApp(s, appName);
+        if (wac == null)
+            return false;
+        return wac.isStarted();
+    }
+    
+    /**
+     *  Warning, this will NOT work during shutdown, because
+     *  the console is already unregistered.
+     *
+     *  @since Jetty 6
+     */
     static ContextHandler getWebApp(I2PAppContext ctx, String appName) {
         ContextHandlerCollection server = getConsoleServer(ctx);
         if (server == null)
             return null;
+        return getWebApp(server, appName);
+    }
+    
+    /**
+     *  @since 0.9.41
+     */
+    static ContextHandler getWebApp(Server s, String appName) {
+        ContextHandlerCollection server = getConsoleServer(s);
+        if (server == null)
+            return null;
+        return getWebApp(server, appName);
+    }
+    
+    /**
+     *  @since 0.9.41
+     */
+    private static ContextHandler getWebApp(ContextHandlerCollection server, String appName) {
         Handler handlers[] = server.getHandlers();
         if (handlers == null)
             return null;
@@ -205,12 +267,23 @@ public class WebAppStarter {
 
     /**
      *  See comments in ConfigClientsHandler
+     *
+     *  Warning, this will NOT work during shutdown, because
+     *  the console is already unregistered.
+     *
      *  @since public since 0.9.33, was package private
      */
     public static ContextHandlerCollection getConsoleServer(I2PAppContext ctx) {
         Server s = RouterConsoleRunner.getConsoleServer(ctx);
         if (s == null)
             return null;
+        return getConsoleServer(s);
+    }
+
+    /**
+     *  @since 0.9.41
+     */
+    private static ContextHandlerCollection getConsoleServer(Server s) {
         Handler h = s.getChildHandlerByClass(ContextHandlerCollection.class);
         if (h == null)
             return null;

history.txt

@@ -1,3 +1,21 @@
+2019-06-15 zzz
+ * Console: Fix stopping webapps at shutdown (ticket #2508)
+
+2019-06-09 zzz
+ * Eepsite Help page: Add links to Arabic, Hungarian, Indonesian.
+   fix link to Italian
+ * SusiMail: Add Farsi translation
+ * Tests: Fix some bashisms, add more files to bashisms check
+ * UPnP: Set lease duration of 3 hours, always refresh the lease
+
+2019-06-08 zzz
+ * NetDB:
+   - Fix Deliv. Status msg sent direct to tunnel
+   - Faster startup for non-Android
+
+2019-06-07 zzz
+ * NetDB: Fix NPE on failed decrypt of enc. ls2
+
 2019-06-06 zzz
  * CPUID/NBI:
    - Add Skylake support (ticket #1869)

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 = "";

router/java/src/net/i2p/router/startup/RouterAppManager.java

@@ -129,7 +129,10 @@ public class RouterAppManager extends ClientAppManagerImpl {
           case STOPPING:
           case STOPPED:
             _clients.remove(app);
-            _registered.remove(app.getName(), app);
+            boolean removed = _registered.remove(app.getName(), app);
+            if (removed && _log.shouldInfo()) {
+                _log.info("Client " + app.getDisplayName() + " UNREGISTERED AS " + app.getName());
+            }
             if (message == null)
                 message = "";
             if (_log.shouldLog(Log.INFO))
@@ -140,7 +143,10 @@ public class RouterAppManager extends ClientAppManagerImpl {
           case CRASHED:
           case START_FAILED:
             _clients.remove(app);
-            _registered.remove(app.getName(), app);
+            boolean removed2 = _registered.remove(app.getName(), app);
+            if (removed2 && _log.shouldInfo()) {
+                _log.info("Client " + app.getDisplayName() + " UNREGISTERED AS " + app.getName());
+            }
             if (message == null)
                 message = "";
             _log.log(Log.CRIT, "Client " + app.getDisplayName() + ' ' + state +
@@ -172,6 +178,22 @@ public class RouterAppManager extends ClientAppManagerImpl {
         return super.register(app);
     }
     
+    /**
+     *  Unregister with the manager. Name must be the same as that from register().
+     *  Only required for apps used by other apps.
+     *
+     *  @param app non-null
+     *  @since 0.9.41 overridden for logging only
+     */
+    @Override
+    public void unregister(ClientApp app) {
+        if (_log.shouldInfo()) {
+            if (getRegisteredApp(app.getName()) != null)
+                _log.info("Client " + app.getDisplayName() + " UNREGISTERED AS " + app.getName());
+        }
+        super.unregister(app);
+    }
+
     /// end ClientAppManager interface
 
     /**