From a132582db7f3718d51959cd093bea5198bf94530 Mon Sep 17 00:00:00 2001 From: Joey Sullivan Date: Thu, 5 Oct 2017 19:48:12 +0000 Subject: 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 --- .../openecomp/appc/oam/processor/BaseCommon.java | 8 +++++- .../appc/oam/processor/BaseProcessor.java | 29 ++++++++++++++-------- .../openecomp/appc/oam/util/AsyncTaskHelper.java | 6 ++++- .../appc/oam/util/ConfigurationHelper.java | 8 +++--- 4 files changed, 36 insertions(+), 15 deletions(-) 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 9c28007b4..0cfab98b4 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.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 aa5423d3b..41955e135 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 @@ 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 0a4b868a8..9c344c14e 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 @@ 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(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 6e6ab036b..25df3dfd9 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 @@ 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 + ); } } -- cgit 1.2.3-korg