APPC - OAM ConcurrentModificationException

Fixes:

- In the AsyncTaskHelper, the cancel future augmentation in the
scheduleBaseRunnable(...) removed itself from the myFutureSet while
it was iterating over the entries. This caused the
ConcurrentModificationException

- The Timeout operation in the ConfigurationHelper was not converting
the units to millisecond.

- The Timeout in the BaseProcessor. preProcess(...) was NOT using the
request-timeout value supplied in the request.

- When a timeout occurred in the BaseProcessor. preProcess(...) method,
the OAM state was not being switched to ERROR state

Issue-Id: APPC-263
Change-Id: I02c5e3adaca9a595d2c3541d8a997f3254bf097a
Signed-off-by: Joey Sullivan <joey.sullivan@amdocs.com>

diff --git a/appc-oam/appc-oam-bundle/src/main/java/org/openecomp/appc/oam/processor/BaseCommon.java b/appc-oam/appc-oam-bundle/src/main/java/org/openecomp/appc/oam/processor/BaseCommon.java
index 9c28007..0cfab98 100644 (file)
--- a/appc-oam/appc-oam-bundle/src/main/java/org/openecomp/appc/oam/processor/BaseCommon.java
+++ b/appc-oam/appc-oam-bundle/src/main/java/org/openecomp/appc/oam/processor/BaseCommon.java
@@ -43,11 +43,13 @@ import org.openecomp.appc.oam.util.StateHelper;
 import org.slf4j.MDC;
 
 import java.net.InetAddress;
+import java.time.Instant;
 import java.util.Arrays;
 import java.util.Date;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.TimeoutException;
 
 import static com.att.eelf.configuration.Configuration.MDC_INSTANCE_UUID;
 import static com.att.eelf.configuration.Configuration.MDC_KEY_REQUEST_ID;
@@ -107,7 +109,7 @@ public abstract class BaseCommon {
      */
     void auditInfoLog(Msg msg) {
         LoggingUtils.auditInfo(startTime.toInstant(),
-                new Date(System.currentTimeMillis()).toInstant(),
+            Instant.now(),
             String.valueOf(status.getCode()),
             status.getMessage(),
             getClass().getCanonicalName(),
@@ -227,6 +229,10 @@ public abstract class BaseCommon {
                 rpc.getAppcOperation(), appName, stateHelper.getCurrentOamState());
             oamCommandStatus = OAMCommandStatus.REJECTED;
             errorMessage = EELFResourceManager.format(Msg.INVALID_STATE_TRANSITION, exceptionMessage);
+        } else if (t instanceof TimeoutException) {
+            oamCommandStatus = OAMCommandStatus.TIMEOUT;
+            errorMessage = EELFResourceManager.format(Msg.OAM_OPERATION_EXCEPTION, t,
+                    appName, t.getClass().getSimpleName(), rpc.name(), exceptionMessage);
         } else {
             oamCommandStatus = OAMCommandStatus.UNEXPECTED_ERROR;
             errorMessage = EELFResourceManager.format(Msg.OAM_OPERATION_EXCEPTION, t,

diff --git a/appc-oam/appc-oam-bundle/src/main/java/org/openecomp/appc/oam/processor/BaseProcessor.java b/appc-oam/appc-oam-bundle/src/main/java/org/openecomp/appc/oam/processor/BaseProcessor.java
index aa5423d..41955e1 100644 (file)
--- a/appc-oam/appc-oam-bundle/src/main/java/org/openecomp/appc/oam/processor/BaseProcessor.java
+++ b/appc-oam/appc-oam-bundle/src/main/java/org/openecomp/appc/oam/processor/BaseProcessor.java
@@ -95,8 +95,6 @@ public abstract class BaseProcessor extends BaseCommon {
 
         try {
             preProcess(requestInput);
-            //The OAM request may specify timeout value
-            requestTimeoutSeconds = operationHelper.getParamRequestTimeout(requestInput);
             scheduleAsyncTask();
         } catch (Exception e) {
             setErrorStatus(e);
@@ -122,6 +120,9 @@ public abstract class BaseProcessor extends BaseCommon {
         setInitialLogProperties();
         operationHelper.isInputValid(requestInput);
 
+        //The OAM request may specify timeout value
+        requestTimeoutSeconds = operationHelper.getParamRequestTimeout(requestInput);
+
         //All OAM operation pass through here first to validate if an OAM state change is allowed.
         //If a state change is allowed cancel the occurring OAM (if any) before starting this one.
         //we will synchronized so that only one can do this at any given time.
@@ -134,14 +135,22 @@ public abstract class BaseProcessor extends BaseCommon {
 
             stateHelper.setState(nextState);
 
-            //cancel the  BaseActionRunnable currently executing
-            //it got to be completely terminated before proceeding
-            asyncTaskHelper.cancelBaseActionRunnable(
-                    rpc,
-                    currentOamState,
-                    getTimeoutMilliseconds(),
-                    TimeUnit.MILLISECONDS
-            );
+
+            try {
+                //cancel the  BaseActionRunnable currently executing
+                //it got to be completely terminated before proceeding
+                asyncTaskHelper.cancelBaseActionRunnable(
+                        rpc,
+                        currentOamState,
+                        getTimeoutMilliseconds(),
+                        TimeUnit.MILLISECONDS
+                );
+            } catch (TimeoutException e) {
+                stateHelper.setState(AppcOamStates.Error);
+                throw e;
+            }
+
+
         }
     }
 

diff --git a/appc-oam/appc-oam-bundle/src/main/java/org/openecomp/appc/oam/util/AsyncTaskHelper.java b/appc-oam/appc-oam-bundle/src/main/java/org/openecomp/appc/oam/util/AsyncTaskHelper.java
index 0a4b868..9c344c1 100644 (file)
--- a/appc-oam/appc-oam-bundle/src/main/java/org/openecomp/appc/oam/util/AsyncTaskHelper.java
+++ b/appc-oam/appc-oam-bundle/src/main/java/org/openecomp/appc/oam/util/AsyncTaskHelper.java
@@ -201,7 +201,11 @@ public class AsyncTaskHelper {
                 boolean cancel;
                 synchronized (AsyncTaskHelper.this) {
                     cancel = super.cancel(mayInterruptIfRunning);
-                    myFutureSet.stream().filter(f->!this.equals(f)).forEach(f->f.cancel(mayInterruptIfRunning));
+                    //clone the set to prevent java.util.ConcurrentModificationException.  The  synchronized prevents
+                    //other threads from modifying this set, but not itself.  The  f->f.cancel may modify myFutureSet by
+                    //removing an entry which breaks the iteration in the forEach.
+                    (new HashSet<MyFuture>(myFutureSet))
+                            .stream().filter(f->!this.equals(f)).forEach(f->f.cancel(mayInterruptIfRunning));
                 }
                 return cancel;
             }

diff --git a/appc-oam/appc-oam-bundle/src/main/java/org/openecomp/appc/oam/util/ConfigurationHelper.java b/appc-oam/appc-oam-bundle/src/main/java/org/openecomp/appc/oam/util/ConfigurationHelper.java
index 6e6ab03..25df3df 100644 (file)
--- a/appc-oam/appc-oam-bundle/src/main/java/org/openecomp/appc/oam/util/ConfigurationHelper.java
+++ b/appc-oam/appc-oam-bundle/src/main/java/org/openecomp/appc/oam/util/ConfigurationHelper.java
@@ -96,9 +96,11 @@ public class ConfigurationHelper {
      * @return timeout in milliseconds
      */
     public long getOAMOperationTimeoutValue(Integer overrideTimeoutSeconds) {
-        return overrideTimeoutSeconds == null ?
-            getConfig().getIntegerProperty(OAM_OPERATION_TIMEOUT_SECOND, DEFAULT_OAM_OPERATION_TIMEOUT) * 1000
+        return TimeUnit.SECONDS.toMillis(
+                overrideTimeoutSeconds == null ?
+            getConfig().getIntegerProperty(OAM_OPERATION_TIMEOUT_SECOND, DEFAULT_OAM_OPERATION_TIMEOUT)
             :
-            TimeUnit.MILLISECONDS.toMillis(overrideTimeoutSeconds);
+            overrideTimeoutSeconds
+        );
     }
 }