Merge "Fix hanging timeout on Executor test"

3 files changed, 24 insertions, 12 deletions

diff --git a/plugins/plugins-executor/plugins-executor-javascript/pom.xml b/plugins/plugins-executor/plugins-executor-javascript/pom.xml
index 007a03ae2..80fe06c79 100644
--- a/plugins/plugins-executor/plugins-executor-javascript/pom.xml
+++ b/plugins/plugins-executor/plugins-executor-javascript/pom.xml
@@ -59,7 +59,6 @@
             <artifactId>awaitility</artifactId>
             <scope>test</scope>
         </dependency>
-
     </dependencies>
 
     <profiles>
diff --git a/plugins/plugins-executor/plugins-executor-javascript/src/main/java/org/onap/policy/apex/plugins/executor/javascript/JavascriptExecutor.java b/plugins/plugins-executor/plugins-executor-javascript/src/main/java/org/onap/policy/apex/plugins/executor/javascript/JavascriptExecutor.java
index a33a129af..489489ff0 100644
--- a/plugins/plugins-executor/plugins-executor-javascript/src/main/java/org/onap/policy/apex/plugins/executor/javascript/JavascriptExecutor.java
+++ b/plugins/plugins-executor/plugins-executor-javascript/src/main/java/org/onap/policy/apex/plugins/executor/javascript/JavascriptExecutor.java
@@ -50,8 +50,12 @@ public class JavascriptExecutor implements Runnable {
 
     public static final int DEFAULT_OPTIMIZATION_LEVEL = 9;
 
+    // Token passed to executor thread to stop execution
+    private static final Object STOP_EXECUTION_TOKEN = "*** STOP EXECUTION ***";
+
     // Recurring string constants
     private static final String WITH_MESSAGE = " with message: ";
+    private static final String JAVASCRIPT_EXECUTOR = "JavascriptExecutor ";
 
     @Setter(AccessLevel.PROTECTED)
     private static TimeUnit timeunit4Latches = TimeUnit.SECONDS;
@@ -120,7 +124,7 @@ public class JavascriptExecutor implements Runnable {
         try {
             if (!intializationLatch.await(intializationLatchTimeout, timeunit4Latches)) {
                 executorThread.interrupt();
-                throw new StateMachineException("JavascriptExecutor " + subjectKey.getId()
+                throw new StateMachineException(JAVASCRIPT_EXECUTOR + subjectKey.getId()
                     + " initiation timed out after " + intializationLatchTimeout + " " + timeunit4Latches);
             }
         } catch (InterruptedException e) {
@@ -160,7 +164,7 @@ public class JavascriptExecutor implements Runnable {
             executorThread.interrupt();
             Thread.currentThread().interrupt();
             throw new StateMachineException(
-                "JavascriptExecutor " + subjectKey.getId() + "interrupted on execution result wait", e);
+                JAVASCRIPT_EXECUTOR + subjectKey.getId() + "interrupted on execution result wait", e);
         }
 
         checkAndThrowExecutorException();
@@ -179,11 +183,11 @@ public class JavascriptExecutor implements Runnable {
         }
 
         if (executorThread.isAlive()) {
-            executorThread.interrupt();
+            executionQueue.add(STOP_EXECUTION_TOKEN);
 
             try {
                 if (!cleanupLatch.await(cleanupLatchTimeout, timeunit4Latches)) {
-                    executorException.set(new StateMachineException("JavascriptExecutor " + subjectKey.getId()
+                    executorException.set(new StateMachineException(JAVASCRIPT_EXECUTOR + subjectKey.getId()
                         + " cleanup timed out after " + cleanupLatchTimeout + " " + timeunit4Latches));
                 }
             } catch (InterruptedException e) {
@@ -221,12 +225,14 @@ public class JavascriptExecutor implements Runnable {
         while (!Thread.currentThread().isInterrupted()) {
             try {
                 Object contextObject = executionQueue.take();
-
-                boolean result = executeScript(contextObject);
-                resultQueue.add(result);
+                if (STOP_EXECUTION_TOKEN.equals(contextObject)) {
+                    LOGGER.debug("execution close was ordered for  " + subjectKey.getId());
+                    break;
+                }
+                resultQueue.add(executeScript(contextObject));
             } catch (final InterruptedException e) {
                 LOGGER.debug("execution was interruped for " + subjectKey.getId() + WITH_MESSAGE + e.getMessage(), e);
-                resultQueue.add(false);
+                executionQueue.add(STOP_EXECUTION_TOKEN);
                 Thread.currentThread().interrupt();
             } catch (StateMachineException sme) {
                 executorException.set(sme);
@@ -234,6 +240,8 @@ public class JavascriptExecutor implements Runnable {
             }
         }
 
+        resultQueue.add(false);
+
         try {
             Context.exit();
         } catch (final Exception e) {
diff --git a/plugins/plugins-executor/plugins-executor-javascript/src/test/java/org/onap/policy/apex/plugins/executor/javascript/JavascriptExecutorTest.java b/plugins/plugins-executor/plugins-executor-javascript/src/test/java/org/onap/policy/apex/plugins/executor/javascript/JavascriptExecutorTest.java
index 53781e870..6ea15fc35 100644
--- a/plugins/plugins-executor/plugins-executor-javascript/src/test/java/org/onap/policy/apex/plugins/executor/javascript/JavascriptExecutorTest.java
+++ b/plugins/plugins-executor/plugins-executor-javascript/src/test/java/org/onap/policy/apex/plugins/executor/javascript/JavascriptExecutorTest.java
@@ -267,7 +267,7 @@ public class JavascriptExecutorTest {
         }).doesNotThrowAnyException();
 
         assertThatCode(() -> {
-            executor.init("while (true) { x = 1; }; true;");
+            executor.init("var x = 0; while (x < 100) { x++; }; true;");
         }).doesNotThrowAnyException();
 
         concurrentResult.set(true);
@@ -276,13 +276,18 @@ public class JavascriptExecutorTest {
         (new Thread() {
             public void run() {
                 try {
-                    concurrentResult.set(executor.execute("hello"));
+                    while (executor.execute("hello")) {
+                        // Loop until interrupted
+                    }
+                    concurrentResult.set(false);
                 } catch (StateMachineException e) {
-                    e.printStackTrace();
+                    // Do nothing
                 }
             }
         }).start();
 
+        await().atMost(1000, TimeUnit.MILLISECONDS).until(() -> executor.getExecutorThread().isAlive());
+
         executor.getExecutorThread().interrupt();
 
         await().atMost(1000, TimeUnit.MILLISECONDS).until(() -> !concurrentResult.get());