From 0816cfe2733ad3f3bc8ff2c8ba4dda2a867b0035 Mon Sep 17 00:00:00 2001
From: zzz <zzz@mail.i2p>
Date: Fri, 26 Apr 2013 16:41:09 +0000
Subject: [PATCH]  * Plugins: Track pending plugin clients better, don't hold
 references,             start delayed clients from SimpleTimer2 instead of
 Job queue (ticket #670)

---
 .../src/net/i2p/router/web/PluginStarter.java | 89 +++++++++++++------
 history.txt                                   |  2 +
 router/java/src/net/i2p/router/Job.java       |  1 +
 router/java/src/net/i2p/router/JobImpl.java   |  2 +
 router/java/src/net/i2p/router/JobQueue.java  |  1 +
 router/java/src/net/i2p/router/JobStats.java  |  1 +
 router/java/src/net/i2p/router/JobTiming.java |  1 +
 .../src/net/i2p/router/RouterVersion.java     |  2 +-
 .../i2p/router/startup/LoadClientAppsJob.java | 32 ++++---
 9 files changed, 89 insertions(+), 42 deletions(-)

diff --git a/apps/routerconsole/java/src/net/i2p/router/web/PluginStarter.java b/apps/routerconsole/java/src/net/i2p/router/web/PluginStarter.java
index 5a1ff889a9..08b198dd21 100644
--- a/apps/routerconsole/java/src/net/i2p/router/web/PluginStarter.java
+++ b/apps/routerconsole/java/src/net/i2p/router/web/PluginStarter.java
@@ -20,7 +20,6 @@ import java.util.concurrent.ConcurrentHashMap;
 import net.i2p.CoreVersion;
 import net.i2p.I2PAppContext;
 import net.i2p.data.DataHelper;
-import net.i2p.router.Job;
 import net.i2p.router.RouterContext;
 import net.i2p.router.RouterVersion;
 import net.i2p.router.startup.ClientAppConfig;
@@ -31,6 +30,7 @@ import net.i2p.util.ConcurrentHashSet;
 import net.i2p.util.FileUtil;
 import net.i2p.util.I2PAppThread;
 import net.i2p.util.Log;
+import net.i2p.util.SimpleTimer2;
 import net.i2p.util.Translate;
 import net.i2p.util.VersionComparator;
 
@@ -55,7 +55,8 @@ public class PluginStarter implements Runnable {
     private static final String[] STANDARD_THEMES = { "images", "light", "dark", "classic",
                                                       "midnight" };
     private static Map<String, ThreadGroup> pluginThreadGroups = new ConcurrentHashMap<String, ThreadGroup>();   // one thread group per plugin (map key=plugin name)
-    private static Map<String, Collection<Job>> pluginJobs = new ConcurrentHashMap<String, Collection<Job>>();
+    private static Map<String, Collection<SimpleTimer2.TimedEvent>> _pendingPluginClients =
+                   new ConcurrentHashMap<String, Collection<SimpleTimer2.TimedEvent>>();
     private static Map<String, ClassLoader> _clCache = new ConcurrentHashMap();
     private static Map<String, Collection<String>> pluginWars = new ConcurrentHashMap<String, Collection<String>>();
 
@@ -592,13 +593,13 @@ public class PluginStarter implements Runnable {
     private static void runClientApps(RouterContext ctx, File pluginDir, List<ClientAppConfig> apps, String action) throws Exception {
         Log log = ctx.logManager().getLog(PluginStarter.class);
         
-        // initialize pluginThreadGroup and pluginJobs
+        // initialize pluginThreadGroup and _pendingPluginClients
         String pluginName = pluginDir.getName();
         if (!pluginThreadGroups.containsKey(pluginName))
             pluginThreadGroups.put(pluginName, new ThreadGroup(pluginName));
         ThreadGroup pluginThreadGroup = pluginThreadGroups.get(pluginName);
         if (action.equals("start"))
-            pluginJobs.put(pluginName, new ConcurrentHashSet<Job>());
+            _pendingPluginClients.put(pluginName, new ConcurrentHashSet<SimpleTimer2.TimedEvent>());
         
         for(ClientAppConfig app : apps) {
             if (action.equals("start") && app.disabled)
@@ -675,39 +676,69 @@ public class PluginStarter implements Runnable {
                 try {
                     // quick check
                     LoadClientAppsJob.testClient(app.className, cl);
-               } catch(ClassNotFoundException ex) {
-                   // Try again 1 or 2 seconds later. 
-                   // This should be enough time. Although it is a lousy hack
-                   // it should work for most cases.
-                   // Perhaps it may be even better to delay a percentage
-                   // if > 1, and reduce the delay time.
-                   // Under normal circumstances there will be no delay at all.
-                   if(app.delay > 1) {
-                       Thread.sleep(2000);
-                   } else {
-                       Thread.sleep(1000);
-                   }
-               }
-               // quick check, will throw ClassNotFoundException on error
-               LoadClientAppsJob.testClient(app.className, cl);
-               // wait before firing it up
-               Job job = new LoadClientAppsJob.DelayedRunClient(ctx, app.className, app.clientName, argVal, app.delay, pluginThreadGroup, cl);
-               ctx.jobQueue().addJob(job);
-               pluginJobs.get(pluginName).add(job);
+                } catch (ClassNotFoundException ex) {
+                    // Try again 1 or 2 seconds later. 
+                    // This should be enough time. Although it is a lousy hack
+                    // it should work for most cases.
+                    // Perhaps it may be even better to delay a percentage
+                    // if > 1, and reduce the delay time.
+                    // Under normal circumstances there will be no delay at all.
+                    try {
+                        if (app.delay > 1) {
+                            Thread.sleep(2000);
+                        } else {
+                            Thread.sleep(1000);
+                        }
+                    } catch (InterruptedException ie) {}
+                    // quick check, will throw ClassNotFoundException on error
+                    LoadClientAppsJob.testClient(app.className, cl);
+                }
+                // wait before firing it up
+                SimpleTimer2.TimedEvent evt = new TrackedDelayedClient(pluginName, ctx.simpleTimer2(), ctx, app.className,
+                                                                       app.clientName, argVal, pluginThreadGroup, cl);
+                evt.schedule(app.delay);
             }
         }
     }
 
+    /**
+     *  Simple override to track whether a plugin's client is delayed and queued
+     *  @since 0.9.6
+     */
+    private static class TrackedDelayedClient extends LoadClientAppsJob.DelayedRunClient {
+        private final String _pluginName;
+
+        public TrackedDelayedClient(String pluginName,
+                                    SimpleTimer2 pool, RouterContext enclosingContext, String className, String clientName,
+                                    String args[], ThreadGroup threadGroup, ClassLoader cl) {
+            super(pool, enclosingContext, className, clientName, args, threadGroup, cl);
+            _pluginName = pluginName;
+            _pendingPluginClients.get(pluginName).add(this);
+        }
+
+        @Override
+        public boolean cancel() {
+            boolean rv = super.cancel();
+            _pendingPluginClients.get(_pluginName).remove(this);
+            return rv;
+        }
+
+        @Override
+        public void timeReached() {
+            super.timeReached();
+            _pendingPluginClients.get(_pluginName).remove(this);
+        }
+    }
+
     public static boolean isPluginRunning(String pluginName, RouterContext ctx) {
         Log log = ctx.logManager().getLog(PluginStarter.class);
         
         boolean isJobRunning = false;
-        if (pluginJobs.containsKey(pluginName))
-            for (Job job: pluginJobs.get(pluginName))
-                if (ctx.jobQueue().isJobActive(job)) {
-                    isJobRunning = true;
-                    break;
-                }
+        Collection<SimpleTimer2.TimedEvent> pending = _pendingPluginClients.get(pluginName);
+        if (pending != null && !pending.isEmpty()) {
+            // TODO have a pending indication too
+            isJobRunning = true;
+        }
         boolean isWarRunning = false;
         if(pluginWars.containsKey(pluginName)) {
             Iterator <String> it = pluginWars.get(pluginName).iterator();
diff --git a/history.txt b/history.txt
index 036dc8ad8d..fa453c1214 100644
--- a/history.txt
+++ b/history.txt
@@ -2,6 +2,8 @@
  * Console: Show log location on /logs even if not opened yet (ticket #905)
  * HTTP proxy: Verify nonce count in digest auth
  * i2psnark: Use smaller piece size for small torrents
+ * Plugins: Track pending plugin clients better, don't hold references,
+            start delayed clients from SimpleTimer2 instead of Job queue (ticket #670)
 
 2013-04-25 kytv
  * Portuguese, Russian, Spanish, and Swedish translation updates from Transifex
diff --git a/router/java/src/net/i2p/router/Job.java b/router/java/src/net/i2p/router/Job.java
index 6c85c1af46..9ca34fff51 100644
--- a/router/java/src/net/i2p/router/Job.java
+++ b/router/java/src/net/i2p/router/Job.java
@@ -12,6 +12,7 @@ package net.i2p.router;
 /**
  * Defines an executable task
  *
+ * For use by the router only. Not to be used by applications or plugins.
  */
 public interface Job {
     /**
diff --git a/router/java/src/net/i2p/router/JobImpl.java b/router/java/src/net/i2p/router/JobImpl.java
index 9bfd30b397..21ee8f9d7d 100644
--- a/router/java/src/net/i2p/router/JobImpl.java
+++ b/router/java/src/net/i2p/router/JobImpl.java
@@ -12,6 +12,8 @@ import java.util.concurrent.atomic.AtomicLong;
 
 /**
  * Base implementation of a Job
+ *
+ * For use by the router only. Not to be used by applications or plugins.
  */
 public abstract class JobImpl implements Job {
     private final RouterContext _context;
diff --git a/router/java/src/net/i2p/router/JobQueue.java b/router/java/src/net/i2p/router/JobQueue.java
index 813d5768fe..913e7a544e 100644
--- a/router/java/src/net/i2p/router/JobQueue.java
+++ b/router/java/src/net/i2p/router/JobQueue.java
@@ -31,6 +31,7 @@ import net.i2p.util.Log;
  * Manage the pending jobs according to whatever algorithm is appropriate, giving
  * preference to earlier scheduled jobs.
  *
+ * For use by the router only. Not to be used by applications or plugins.
  */
 public class JobQueue {
     private final Log _log;
diff --git a/router/java/src/net/i2p/router/JobStats.java b/router/java/src/net/i2p/router/JobStats.java
index eadcbf2359..446465154d 100644
--- a/router/java/src/net/i2p/router/JobStats.java
+++ b/router/java/src/net/i2p/router/JobStats.java
@@ -5,6 +5,7 @@ import net.i2p.data.DataHelper;
 /**
  *  Glorified struct to contain basic job stats.
  *  Public for router console only.
+ *  For use by the router only. Not to be used by applications or plugins.
  */
 public class JobStats {
     private final String _job;
diff --git a/router/java/src/net/i2p/router/JobTiming.java b/router/java/src/net/i2p/router/JobTiming.java
index 599f17d2eb..818203aeb9 100644
--- a/router/java/src/net/i2p/router/JobTiming.java
+++ b/router/java/src/net/i2p/router/JobTiming.java
@@ -13,6 +13,7 @@ import net.i2p.util.Clock;
 /**
  * Define the timing requirements and statistics for a particular job
  *
+ * For use by the router only. Not to be used by applications or plugins.
  */
 public class JobTiming implements Clock.ClockUpdateListener {
     private long _start;
diff --git a/router/java/src/net/i2p/router/RouterVersion.java b/router/java/src/net/i2p/router/RouterVersion.java
index e1f87e077d..8ab24107c9 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 = 16;
+    public final static long BUILD = 17;
 
     /** for example "-test" */
     public final static String EXTRA = "";
diff --git a/router/java/src/net/i2p/router/startup/LoadClientAppsJob.java b/router/java/src/net/i2p/router/startup/LoadClientAppsJob.java
index 92e47a5f9b..e31176d840 100644
--- a/router/java/src/net/i2p/router/startup/LoadClientAppsJob.java
+++ b/router/java/src/net/i2p/router/startup/LoadClientAppsJob.java
@@ -14,6 +14,7 @@ import net.i2p.router.RouterContext;
 import net.i2p.router.app.RouterApp;
 import net.i2p.util.I2PThread;
 import net.i2p.util.Log;
+import net.i2p.util.SimpleTimer2;
 
 /**
  * Run any client applications specified in clients.config.  If any clientApp
@@ -51,12 +52,18 @@ public class LoadClientAppsJob extends JobImpl {
                 runClient(app.className, app.clientName, argVal, getContext(), _log);
             } else {
                 // wait before firing it up
-                getContext().jobQueue().addJob(new DelayedRunClient(getContext(), app.className, app.clientName, argVal, app.delay));
+                DelayedRunClient drc = new DelayedRunClient(getContext().simpleTimer2(), getContext(), app.className,
+                                                            app.clientName, argVal);
+                drc.schedule(app.delay);
             }
         }
     }
 
-    public static class DelayedRunClient extends JobImpl {
+    /**
+     *  Public for router console only, not for use by others, subject to change
+     */
+    public static class DelayedRunClient extends SimpleTimer2.TimedEvent {
+        private final RouterContext _ctx;
         private final String _className;
         private final String _clientName;
         private final String _args[];
@@ -64,26 +71,27 @@ public class LoadClientAppsJob extends JobImpl {
         private final ThreadGroup _threadGroup;
         private final ClassLoader _cl;
 
-        public DelayedRunClient(RouterContext enclosingContext, String className, String clientName, String args[], long delay) {
-            this(enclosingContext, className, clientName, args, delay, null, null);
+        /** caller MUST call schedule() */
+        public DelayedRunClient(SimpleTimer2 pool, RouterContext enclosingContext, String className,
+                                String clientName, String args[]) {
+            this(pool, enclosingContext, className, clientName, args, null, null);
         }
         
-        public DelayedRunClient(RouterContext enclosingContext, String className, String clientName, String args[],
-                                long delay, ThreadGroup threadGroup, ClassLoader cl) {
-            super(enclosingContext);
+        /** caller MUST call schedule() */
+        public DelayedRunClient(SimpleTimer2 pool, RouterContext enclosingContext, String className, String clientName,
+                                String args[], ThreadGroup threadGroup, ClassLoader cl) {
+            super(pool);
+            _ctx = enclosingContext;
             _className = className;
             _clientName = clientName;
             _args = args;
             _log = enclosingContext.logManager().getLog(LoadClientAppsJob.class);
             _threadGroup = threadGroup;
             _cl = cl;
-            getTiming().setStartAfter(getContext().clock().now() + delay);
         }
 
-        public String getName() { return "Delayed client job"; }
-
-        public void runJob() {
-            runClient(_className, _clientName, _args, getContext(), _log, _threadGroup, _cl);
+        public void timeReached() {
+            runClient(_className, _clientName, _args, _ctx, _log, _threadGroup, _cl);
         }
     }
     
-- 
GitLab