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
--- 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.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 @@
*/
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 @@
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
--- 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 @@
try {
preProcess(requestInput);
- //The OAM request may specify timeout value
- requestTimeoutSeconds = operationHelper.getParamRequestTimeout(requestInput);
scheduleAsyncTask();
} catch (Exception e) {
setErrorStatus(e);
@@ -122,6 +120,9 @@
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 @@
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
--- 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 @@
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
--- 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 @@
* @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
+ );
}
}