hashtable is much better for quick access in critical sections than iterating over a list of possibilities

src/main/java/JobServ/ProcessManager.java

@@ -17,9 +17,8 @@ import java.util.concurrent.Future;
 class ProcessManager {
     // TODO: LOCK_TIMEOUT should be defined in a configuration management system
     private final int LOCK_TIMEOUT = 5; // seconds
-    private ArrayList<ProcessController> processQueue;
+    private HashTable<int, ProcessController> processQueue;
     private Boolean processQueueMutex = false;
-    private Thread backgroundProcessCleaner;
     private ExecutorService threadPool = Executors.newCachedThreadPool();
 
     private Callable<void> getLockCallable = new Callable<void>() {
@@ -37,12 +36,15 @@ class ProcessManager {
      * initializes process queue and start the background process checking daemon
      */
     public ProcessManager() {
-        processQueue = new ArrayList<ProcessController>();
+        processQueue = new HashTable<int, ProcessController>();
-        // TODO: In a long running server over a large period of time
+        /* TODO: In a long running server over a large period of time
-        // It is possible that the streams used to redirect IO in the
+         * It is possible that the streams used to redirect IO in the
-        // Processes may become a significant use of resources.
+         * Processes may become a significant use of resources.
-        // In this case a background thread should be called to periodically
+         * In this case a background thread should be called to periodically
-        // remove dead ProcessControllers after calling kill() on them.
+         * remove dead ProcessControllers after calling kill() on them.
+         *
+         * (grab lock, iterate over map, remove processes that are done executing, store exit codes, release lock, sleep, repeat)
+         */
     }
 
     /*
@@ -70,7 +72,7 @@ class ProcessManager {
         }
 
         ProcessController newProc = ProcessController(command);
-        this.processQueue.add(newProc);
+        this.processes.map(newProc.getPid(), newProc);
 
         // Exit critical section
         this.releaseLock();
@@ -87,6 +89,8 @@ class ProcessManager {
      * 3: couldnt grab lock
      */
     public int getProcessStatus(int pid) {
+        int status;
+
         // Enter critical section
         try {
             this.getLock();
@@ -98,12 +102,11 @@ class ProcessManager {
             return 3;
         }
 
-        for (ProcessController iter : this.processQueue) {
+        ProcessController candidate = this.processMap.get(pid);
-            if (iter.getPid() == pid) {
+        if (candidate != null) {
-                this.releaseLock();
+            status = candidate.getStatus();
-                // release lock on finding process
+            this.releaseLock();
-                return iter.getStatus();
+            return status;
-            }
         }
 
         // process must not exist
@@ -118,6 +121,8 @@ class ProcessManager {
      * a 258 if process doesnt exist.
      */
     public int getProcessReturn(int pid) {
+        int ret;
+
         // Enter Critical section
         try {
             this.getLock();
@@ -128,11 +133,11 @@ class ProcessManager {
             return 257;
         }
 
-        for (ProcessController iter : this.processQueue) {
+        ProcessController candidate = this.processMap.get(pid);
-            if (iter.getPid() == pid) {
+        if (candidate != null) {
-                this.releaseLock();
+            ret = candidate.getReturn();
-                return iter.getReturn();
+            this.releaseLock();
-            }
+            return ret;
         }
 
         this.releaseLock();
@@ -145,6 +150,8 @@ class ProcessManager {
      * or returns description of error
      */
     public String getProcessOutput(int pid) {
+        String output;
+
         try {
             this.getLock();
 
@@ -154,12 +161,11 @@ class ProcessManager {
             return "[-] ERROR: Timeout grabbing lock to access process information";
         }
 
-        for (ProcessController iter : this.processQueue) {
+        ProcessController candidate = this.processMap.get(pid);
-            if (iter.getPid() == pid) {
+        if (candidate != null) {
-                output = iter.getOutput();
+            output = iter.getOutput();
-                this.releaseLock();
+            this.releaseLock();
-                return output;
+            return output;
-            }
         }
 
         this.releaseLock();
@@ -180,42 +186,15 @@ class ProcessManager {
             return false;
         }
 
-        for (ProcessController iter : this.processQueue) {
+        ProcessController candidate = this.processMap.get(pid);
-            if (iter.getPid() == pid) {
+        if (candidate != null) {
-                iter.kill();
+            candidate.kill();
-                break;
-            }
         }
 
         this.releaseLock();
         return true;
     }
 
-    /*
-     * cleanProcessQueue()
-     * represents a background thread that sits and cleans finished processes
-     */
-    private void cleanProcessQueue() {
-        while(true){
-            try {
-                this.getLock();
-
-            } catch (TimeoutException e) {
-                continue;
-            }
-
-            for (ProcessController iter : this.processQueue) {
-                if(!iter.getStatus()) {
-                    iter.kill();
-                    this.processQueue.remove(iter);
-                }
-            }
-
-            this.releaseLock();
-            Thread.sleep(5000);
-        }
-    }
-
     /*
      * getLock()
      * Locks access to this.processQueue