diff --git a/src/main/java/JobServ/ProcessController.java b/src/main/java/JobServ/ProcessController.java index bceb64c..2b69c51 100644 --- a/src/main/java/JobServ/ProcessController.java +++ b/src/main/java/JobServ/ProcessController.java @@ -8,6 +8,11 @@ package JobServ; +import java.util.Scanner; +import java.io.InputStream; +import java.io.OutputStream; +import java.io.IOException; + /* * ProcessController * This class wraps a java Process object with metadata @@ -36,11 +41,11 @@ class ProcessController { this.pid = ProcessController.nextPid; ProcessController.nextPid += 1; - this.process = Runtime.exec(command); + this.process = Runtime.getRuntime().exec(command); this.output = this.process.getOutputStream(); this.input = this.process.getInputStream(); this.outputScanner = new Scanner(this.input); - this.outputScanner.useDelimieter("\\A"); + this.outputScanner.useDelimiter("\\A"); } /* @@ -60,12 +65,12 @@ class ProcessController { * * TODO: (for future release) return thread state */ - public Boolean getStatus() { + public int getStatus() { try { process.exitValue(); - return true; + return 1; } catch (IllegalThreadStateException e) { - return false; + return 0; } } @@ -89,8 +94,8 @@ class ProcessController { */ public String getOutput() { String out = ""; - while(scanner.hasNext()) { - out += scanner.next(); + while(outputScanner.hasNext()) { + out += outputScanner.next(); } return out; @@ -101,8 +106,13 @@ class ProcessController { * Cleans up resources and destroys process */ public void kill() { - this.input.close(); - this.output.close(); + try { + this.input.close(); + this.output.close(); + } catch (IOException e) { + // streams already closed + } + process.destroy(); } } diff --git a/src/main/java/JobServ/ProcessManager.java b/src/main/java/JobServ/ProcessManager.java index bb8f979..7e53e58 100644 --- a/src/main/java/JobServ/ProcessManager.java +++ b/src/main/java/JobServ/ProcessManager.java @@ -9,41 +9,52 @@ package JobServ; import java.util.concurrent.Future; +import java.util.concurrent.Callable; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.Executors; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.TimeoutException; +import java.util.concurrent.ExecutionException; +import java.util.HashMap; +import java.util.Iterator; +import java.io.IOException; /* + * ProcessManager * Holds a list of ProcessControllers and controls access to them via mutex - * Additionally, starts and manages a background thread that clears finished processes from the arraylist + * Mutex Timeout is declared here as well. */ class ProcessManager { // TODO: LOCK_TIMEOUT should be defined in a configuration management system private final int LOCK_TIMEOUT = 5; // seconds - private HashTable processQueue; + private HashMap processMap; private Boolean processQueueMutex = false; private ExecutorService threadPool = Executors.newCachedThreadPool(); - private Callable getLockCallable = new Callable() { - public void Object call() { + private Callable lockCallable = new Callable() { + public Object call() { while(processQueueMutex){ continue; // spin! } processQueueMutex = true; + return 1; } - } + }; /* * Constructor * initializes process queue and start the background process checking daemon */ public ProcessManager() { - processQueue = new HashTable(); + processMap = new HashMap(); /* TODO: In a long running server over a large period of time * It is possible that the streams used to redirect IO in the * Processes may become a significant use of resources. * In this case a background thread should be called to periodically * 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) + * (grab lock, iterate over map, remove finished processes, store exit codes, release lock, sleep, repeat) */ } @@ -51,6 +62,7 @@ class ProcessManager { * newProcess() * Takes a command and returns the translated pid of a new process * Returns -1 if getLock fails + * Returns -2 if controller throws an IOException */ public int newProcess(String command) { /* @@ -61,23 +73,29 @@ class ProcessManager { * which likely changed system state before it was killed. */ - // Enter critical section try { + // Enter critical section this.getLock(); + ProcessController newProc = new ProcessController(command); + this.processMap.put(newProc.getPid(), newProc); + + // Exit critical section + this.releaseLock(); + + return newProc.getPid(); + } catch (TimeoutException e) { // (lock was not grabbed) - System.err.println("Timeout starting new job '%s': " + e.getMessage, command); - return -1 + System.err.println("Timeout starting new job: " + e.getMessage()); + return -1; + + } catch (IOException e) { + // (lock was grabbed) + this.releaseLock(); + System.err.println("ProcessController couldnt start process: " + e.getMessage()); + return -2; } - - ProcessController newProc = ProcessController(command); - this.processes.map(newProc.getPid(), newProc); - - // Exit critical section - this.releaseLock(); - - return newProc.getPid(); } /* @@ -97,8 +115,7 @@ class ProcessManager { } catch (TimeoutException e) { // lock could not be grabbed before timeout - System.err.println("Timeout getting process status for %s: " + e.getMessage(), - Integer.toString(pid)); + System.err.println("Timeout getting process status: " + e.getMessage()); return 3; } @@ -128,8 +145,7 @@ class ProcessManager { this.getLock(); } catch (TimeoutException e) { - System.err.println("Timeout getting process return for %s: " + e.getMessage(), - Integer.toString(pid)); + System.err.println("Timeout getting process return: " + e.getMessage()); return 257; } @@ -156,20 +172,19 @@ class ProcessManager { this.getLock(); } catch (TimeoutException e) { - System.err.println("Timeout getting process output for %s: " + e.getMessage(), - Integer.toString()); + System.err.println("Timeout getting process output: " + e.getMessage()); return "[-] ERROR: Timeout grabbing lock to access process information"; } ProcessController candidate = this.processMap.get(pid); if (candidate != null) { - output = iter.getOutput(); + output = candidate.getOutput(); this.releaseLock(); return output; } this.releaseLock(); - return "[-] ERROR: Process not found" + return "[-] ERROR: Process not found"; } /* @@ -182,7 +197,7 @@ class ProcessManager { this.getLock(); } catch (TimeoutException e) { - System.err.println("Timeout killing process: " + e.getMessage); + System.err.println("Timeout killing process: " + e.getMessage()); return false; } @@ -202,9 +217,10 @@ class ProcessManager { * Throws TimeoutException when it fails to get the lock. */ private synchronized void getLock() throws TimeoutException { + Future future = this.threadPool.submit(this.lockCallable); + try { - Future future = executor.submit(task); - void result = future.get(this.LOCK_TIMEOUT, TimeUnit.SECONDS); + future.get(this.LOCK_TIMEOUT, TimeUnit.SECONDS); } catch (InterruptedException e) { System.err.println("[!] ERROR: " + e.getMessage()); @@ -249,9 +265,14 @@ class ProcessManager { * releases resources held in the processController objects */ private void shutdown() { - this.getLock(); - for (ProcessController p : this.processQueue) { - p.kill(); // exit threads, release IO streams, etc. + this.processQueueMutex = true; + + Iterator> iterator = this.processMap.entrySet().iterator(); + while (iterator.hasNext()) { + HashMap.Entry entry = iterator.next(); + + entry.getValue().kill(); + iterator.remove(); } } } diff --git a/src/test/java/JobServ/JobServerAuthenticationTest.java b/src/test/java/JobServ/JobServerAuthenticationTest.java index 2bec0a9..3a5c099 100644 --- a/src/test/java/JobServ/JobServerAuthenticationTest.java +++ b/src/test/java/JobServ/JobServerAuthenticationTest.java @@ -49,7 +49,7 @@ import org.mockito.ArgumentMatchers; public class JobServerAuthenticationTest { private final String projectRoot = ""; - + // Authorized client key/cert/ca private final String clientCa = projectRoot + "resources/client/ca.crt"; private final String clientKey = projectRoot + "resources/client/private.pem"; @@ -73,65 +73,65 @@ public class JobServerAuthenticationTest { // was setUp able to use SSL Certs private Boolean serverSslInitialized = true; private Boolean clientSslInitialized = true; - + /* * test constructor * generates both clients and the server */ public JobServerAuthenticationTest() throws Exception { - - try { - // generate SSL contexts - SslContextBuilder serverContextBuilder = SslContextBuilder.forServer(new File(serverCert), - new File(serverKey)); - serverContextBuilder.trustManager(new File(clientCa)); - serverContextBuilder.clientAuth(ClientAuth.REQUIRE); - this.server = new JobServServer(GrpcSslContexts.configure(serverContextBuilder).build(), 8448); - this.serverSslInitialized = true; - - } catch (SSLException e) { - this.serverSslInitialized = false; - System.err.println(e.getMessage()); - - } catch (IOException e) { - this.serverSslInitialized = false; - System.err.println(e.getMessage()); - } + try { + // generate SSL contexts + SslContextBuilder serverContextBuilder = SslContextBuilder.forServer(new File(serverCert), + new File(serverKey)); + serverContextBuilder.trustManager(new File(clientCa)); + serverContextBuilder.clientAuth(ClientAuth.REQUIRE); - // generate ssl for clients + this.server = new JobServServer(GrpcSslContexts.configure(serverContextBuilder).build(), 8448); + this.serverSslInitialized = true; + + } catch (SSLException e) { + this.serverSslInitialized = false; + System.err.println(e.getMessage()); + + } catch (IOException e) { + this.serverSslInitialized = false; + System.err.println(e.getMessage()); + } + + // generate ssl for clients if (this.serverSslInitialized) { - try { - SslContextBuilder goodClientBuilder = GrpcSslContexts.forClient(); - goodClientBuilder.trustManager(new File(serverCa)); - goodClientBuilder.keyManager(new File(clientCert), new File(clientKey)); + try { + SslContextBuilder goodClientBuilder = GrpcSslContexts.forClient(); + goodClientBuilder.trustManager(new File(serverCa)); + goodClientBuilder.keyManager(new File(clientCert), new File(clientKey)); + + SslContextBuilder badClientBuilder = GrpcSslContexts.forClient(); + badClientBuilder.trustManager(new File(serverCa)); + badClientBuilder.keyManager(new File(badCert), new File(badKey)); + + ManagedChannel goodChannel = NettyChannelBuilder.forAddress("localhost", 8448) + .sslContext(goodClientBuilder.build()) + .directExecutor() + .build(); - SslContextBuilder badClientBuilder = GrpcSslContexts.forClient(); - badClientBuilder.trustManager(new File(serverCa)); - badClientBuilder.keyManager(new File(badCert), new File(badKey)); - - ManagedChannel goodChannel = NettyChannelBuilder.forAddress("localhost", 8448) - .sslContext(goodClientBuilder.build()) - .directExecutor() - .build(); - - ManagedChannel badChannel = NettyChannelBuilder.forAddress("localhost", 8448) - .sslContext(badClientBuilder.build()) - .directExecutor() - .build(); - - goodClient = new JobServClient(goodChannel); - badClient = new JobServClient(badChannel); - this.clientSslInitialized = true; - - } catch (SSLException e) { - this.clientSslInitialized = false; - System.err.println(e.getMessage()); - } + ManagedChannel badChannel = NettyChannelBuilder.forAddress("localhost", 8448) + .sslContext(badClientBuilder.build()) + .directExecutor() + .build(); + + goodClient = new JobServClient(goodChannel); + badClient = new JobServClient(badChannel); + this.clientSslInitialized = true; + + } catch (SSLException e) { + this.clientSslInitialized = false; + System.err.println(e.getMessage()); + } - } else { - this.clientSslInitialized = false; - } + } else { + this.clientSslInitialized = false; + } } /* @@ -141,14 +141,14 @@ public class JobServerAuthenticationTest { */ @Test public void certificateAuthenticationTest() { - assertEquals(true, serverSslInitialized); - assertEquals(true, clientSslInitialized); + assertEquals(true, serverSslInitialized); + assertEquals(true, clientSslInitialized); - int result = badClient.sendNewJobMessage("test command"); - assertEquals(-2, result); + int result = badClient.sendNewJobMessage("test command"); + assertEquals(-2, result); - result = goodClient.sendNewJobMessage("test command"); - Boolean assertCondition = result == -2; - assertEquals(assertCondition, false); + result = goodClient.sendNewJobMessage("test command"); + Boolean assertCondition = result == -2; + assertEquals(assertCondition, false); } } diff --git a/src/test/java/JobServ/ProcessManagerTest.java b/src/test/java/JobServ/ProcessManagerTest.java new file mode 100644 index 0000000..b6d9b45 --- /dev/null +++ b/src/test/java/JobServ/ProcessManagerTest.java @@ -0,0 +1,49 @@ +/* + * ProcessManagerTest + * + * v1.0 + * + * May 22, 2019 + */ + +package JobServ; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; +import static org.junit.Assert.assertEquals; + + +/* + * ProcessManagerTest + * Class that performs positive and negative unit tests + * of every public method in ProcessManager. This not + * only unit tests ProcessManager but also integration + * tests it with ProcessController. + */ +public class ProcessManagerTest { + ProcessManager manager; + + /* + * ProcessManagerTest constructor + * initializes the process manager + */ + public ProcessManagerTest() { + manager = new ProcessManager(); + } + + /* + * addProcessTest() + * positive unit test for newProcess + */ + @Test + public void addProcessesTest() { + int pid1 = manager.newProcess("ping google"); + assertEquals(0, pid1); + + int pid2 = manager.newProcess("ping google"); + assertEquals(1, pid2); + } + +} +