[Fix] lock action's resource early by locator to stop race condition

This commit is contained in:
Robert von Burg 2018-09-12 10:29:45 +02:00
parent cf9739f091
commit c8d23d0393
7 changed files with 30 additions and 10 deletions

View File

@ -26,6 +26,9 @@ public class ExecuteStoppedActionCommand extends ExecutionCommand {
public void validate() { public void validate() {
DBC.PRE.assertNotNull("action can not be null", this.action); DBC.PRE.assertNotNull("action can not be null", this.action);
tx().lock(this.action.getRootElement());
tx().lock(getResourceLocator(this.action));
if (this.action.getState() != State.STOPPED) { if (this.action.getState() != State.STOPPED) {
String msg = "Action {0} is not in state " + State.STOPPED + " and can thus not be put into execution!"; String msg = "Action {0} is not in state " + State.STOPPED + " and can thus not be put into execution!";
msg = MessageFormat.format(msg, this.action.getState(), State.ERROR, this.action.getLocator()); msg = MessageFormat.format(msg, this.action.getState(), State.ERROR, this.action.getLocator());
@ -37,6 +40,7 @@ public class ExecuteStoppedActionCommand extends ExecutionCommand {
public void doCommand() { public void doCommand() {
Activity rootElement = this.action.getRootElement(); Activity rootElement = this.action.getRootElement();
tx().lock(rootElement); tx().lock(rootElement);
tx().lock(getResourceLocator(this.action));
State currentState = rootElement.getState(); State currentState = rootElement.getState();
rootElement.accept(this); rootElement.accept(this);

View File

@ -9,6 +9,7 @@ import li.strolch.agent.api.ComponentContainer;
import li.strolch.exception.StrolchException; import li.strolch.exception.StrolchException;
import li.strolch.execution.policy.ConfirmationPolicy; import li.strolch.execution.policy.ConfirmationPolicy;
import li.strolch.execution.policy.ExecutionPolicy; import li.strolch.execution.policy.ExecutionPolicy;
import li.strolch.model.Locator;
import li.strolch.model.Order; import li.strolch.model.Order;
import li.strolch.model.Resource; import li.strolch.model.Resource;
import li.strolch.model.State; import li.strolch.model.State;
@ -29,7 +30,7 @@ public abstract class ExecutionCommand extends Command implements TimeOrderingVi
super(container, tx); super(container, tx);
} }
public static Resource getResource(StrolchTransaction tx, Action action) { protected Locator getResourceLocator(Action action) {
String resourceId = action.getResourceId(); String resourceId = action.getResourceId();
if (StringHelper.isEmpty(resourceId) || resourceId.equals(DASH)) if (StringHelper.isEmpty(resourceId) || resourceId.equals(DASH))
throw new StrolchException("No resourceId defined on action " + action.getLocator()); throw new StrolchException("No resourceId defined on action " + action.getLocator());
@ -37,7 +38,18 @@ public abstract class ExecutionCommand extends Command implements TimeOrderingVi
if (StringHelper.isEmpty(resourceType) || resourceType.equals(DASH)) if (StringHelper.isEmpty(resourceType) || resourceType.equals(DASH))
throw new StrolchException("No resourceType defined on action " + action.getLocator()); throw new StrolchException("No resourceType defined on action " + action.getLocator());
return tx.getResourceBy(resourceType, resourceId, true); return Resource.locatorFor(resourceType, resourceId);
}
protected Resource getResource(Action action) {
String resourceId = action.getResourceId();
if (StringHelper.isEmpty(resourceId) || resourceId.equals(DASH))
throw new StrolchException("No resourceId defined on action " + action.getLocator());
String resourceType = action.getResourceType();
if (StringHelper.isEmpty(resourceType) || resourceType.equals(DASH))
throw new StrolchException("No resourceType defined on action " + action.getLocator());
return tx().getResourceBy(resourceType, resourceId, true);
} }
protected void updateOrderState(Activity rootElement, State currentState, State newState) { protected void updateOrderState(Activity rootElement, State currentState, State newState) {
@ -57,13 +69,13 @@ public abstract class ExecutionCommand extends Command implements TimeOrderingVi
} }
protected ExecutionPolicy getExecutionPolicy(Action action) { protected ExecutionPolicy getExecutionPolicy(Action action) {
Resource resource = getResource(tx(), action); Resource resource = getResource(action);
PolicyDef executionPolicyDef = resource.getPolicyDefs().getPolicyDef(ExecutionPolicy.class.getSimpleName()); PolicyDef executionPolicyDef = resource.getPolicyDefs().getPolicyDef(ExecutionPolicy.class.getSimpleName());
return getComponent(PolicyHandler.class).getPolicy(executionPolicyDef, tx()); return getComponent(PolicyHandler.class).getPolicy(executionPolicyDef, tx());
} }
protected ConfirmationPolicy getConfirmationPolicy(Action action) { protected ConfirmationPolicy getConfirmationPolicy(Action action) {
Resource resource = getResource(tx(), action); Resource resource = getResource(action);
PolicyDef executionPolicyDef = resource.getPolicyDefs().getPolicyDef(ConfirmationPolicy.class.getSimpleName()); PolicyDef executionPolicyDef = resource.getPolicyDefs().getPolicyDef(ConfirmationPolicy.class.getSimpleName());
return getComponent(PolicyHandler.class).getPolicy(executionPolicyDef, tx()); return getComponent(PolicyHandler.class).getPolicy(executionPolicyDef, tx());
} }
@ -150,6 +162,8 @@ public abstract class ExecutionCommand extends Command implements TimeOrderingVi
public Void visitAction(Action action) { public Void visitAction(Action action) {
ExecutionPolicy executionPolicy = getExecutionPolicy(action); ExecutionPolicy executionPolicy = getExecutionPolicy(action);
tx().lock(getResourceLocator(action));
if (!executionPolicy.isExecutable(action)) { if (!executionPolicy.isExecutable(action)) {
logger.info("Action " + action.getLocator() + " is not yet executable."); logger.info("Action " + action.getLocator() + " is not yet executable.");
} else { } else {

View File

@ -27,6 +27,7 @@ public class SetActionToErrorCommand extends ExecutionCommand {
DBC.PRE.assertNotNull("action can not be null", this.action); DBC.PRE.assertNotNull("action can not be null", this.action);
tx().lock(this.action.getRootElement()); tx().lock(this.action.getRootElement());
tx().lock(getResourceLocator(this.action));
if (!this.action.getState().canSetToError()) { if (!this.action.getState().canSetToError()) {
String msg = "Current state is {0} and canot be changed to {1} for action {2}"; String msg = "Current state is {0} and canot be changed to {1} for action {2}";
@ -39,6 +40,7 @@ public class SetActionToErrorCommand extends ExecutionCommand {
public void doCommand() { public void doCommand() {
Activity rootElement = this.action.getRootElement(); Activity rootElement = this.action.getRootElement();
tx().lock(rootElement); tx().lock(rootElement);
tx().lock(getResourceLocator(this.action));
if (this.action.getState() == State.ERROR) { if (this.action.getState() == State.ERROR) {
logger.warn("Action " + this.action.getLocator() + " is already in ERROR! Not changing."); logger.warn("Action " + this.action.getLocator() + " is already in ERROR! Not changing.");

View File

@ -27,6 +27,7 @@ public class SetActionToExecutedCommand extends ExecutionCommand {
DBC.PRE.assertNotNull("action can not be null", this.action); DBC.PRE.assertNotNull("action can not be null", this.action);
tx().lock(this.action.getRootElement()); tx().lock(this.action.getRootElement());
tx().lock(getResourceLocator(this.action));
if (!this.action.getState().canSetToExecuted()) { if (!this.action.getState().canSetToExecuted()) {
String msg = "Current state is {0} canot be changed to {1} for action {2}"; String msg = "Current state is {0} canot be changed to {1} for action {2}";
@ -39,6 +40,7 @@ public class SetActionToExecutedCommand extends ExecutionCommand {
public void doCommand() { public void doCommand() {
Activity rootElement = this.action.getRootElement(); Activity rootElement = this.action.getRootElement();
tx().lock(rootElement); tx().lock(rootElement);
tx().lock(getResourceLocator(this.action));
State currentState = rootElement.getState(); State currentState = rootElement.getState();

View File

@ -27,6 +27,7 @@ public class SetActionToStoppedCommand extends ExecutionCommand {
DBC.PRE.assertNotNull("action can not be null", this.action); DBC.PRE.assertNotNull("action can not be null", this.action);
tx().lock(this.action.getRootElement()); tx().lock(this.action.getRootElement());
tx().lock(getResourceLocator(this.action));
if (!this.action.getState().canSetToStopped()) { if (!this.action.getState().canSetToStopped()) {
String msg = "Current state is {0} and canot be changed to {1} for action {2}"; String msg = "Current state is {0} and canot be changed to {1} for action {2}";
@ -39,6 +40,7 @@ public class SetActionToStoppedCommand extends ExecutionCommand {
public void doCommand() { public void doCommand() {
Activity rootElement = this.action.getRootElement(); Activity rootElement = this.action.getRootElement();
tx().lock(rootElement); tx().lock(rootElement);
tx().lock(getResourceLocator(this.action));
if (this.action.getState() == State.STOPPED) { if (this.action.getState() == State.STOPPED) {
logger.warn("Action " + this.action.getLocator() + " is already in STOPPED! Not changing."); logger.warn("Action " + this.action.getLocator() + " is already in STOPPED! Not changing.");

View File

@ -27,6 +27,7 @@ public class SetActionToWarningCommand extends ExecutionCommand {
DBC.PRE.assertNotNull("action can not be null", this.action); DBC.PRE.assertNotNull("action can not be null", this.action);
tx().lock(this.action.getRootElement()); tx().lock(this.action.getRootElement());
tx().lock(getResourceLocator(this.action));
if (!this.action.getState().canSetToWarning()) { if (!this.action.getState().canSetToWarning()) {
String msg = "Current state is {0} and canot be changed to {1} for action {2}"; String msg = "Current state is {0} and canot be changed to {1} for action {2}";
@ -39,6 +40,7 @@ public class SetActionToWarningCommand extends ExecutionCommand {
public void doCommand() { public void doCommand() {
Activity rootElement = this.action.getRootElement(); Activity rootElement = this.action.getRootElement();
tx().lock(rootElement); tx().lock(rootElement);
tx().lock(getResourceLocator(this.action));
if (this.action.getState() == State.WARNING) { if (this.action.getState() == State.WARNING) {
logger.warn("Action " + this.action.getLocator() + " is already in WARNING! Not changing."); logger.warn("Action " + this.action.getLocator() + " is already in WARNING! Not changing.");

View File

@ -41,8 +41,6 @@ public class ReservationExection extends DurationExecution {
@Override @Override
public boolean isExecutable(Action action) { public boolean isExecutable(Action action) {
tx().lock(getResource(action));
// only check if reserve // only check if reserve
if (!action.getType().equals(TYPE_RESERVE) && !action.getType().equals(TYPE_RELEASE)) { if (!action.getType().equals(TYPE_RESERVE) && !action.getType().equals(TYPE_RELEASE)) {
// otherwise delegate to super class // otherwise delegate to super class
@ -71,8 +69,6 @@ public class ReservationExection extends DurationExecution {
@Override @Override
public void toExecution(Action action) { public void toExecution(Action action) {
tx().lock(getResource(action));
// only do if reserve or release // only do if reserve or release
boolean isReserve = action.getType().equals(TYPE_RESERVE); boolean isReserve = action.getType().equals(TYPE_RESERVE);
boolean isRelease = action.getType().equals(TYPE_RELEASE); boolean isRelease = action.getType().equals(TYPE_RELEASE);
@ -95,8 +91,6 @@ public class ReservationExection extends DurationExecution {
@Override @Override
public void toExecuted(Action action) { public void toExecuted(Action action) {
tx().lock(getResource(action));
// only do if reserve or release // only do if reserve or release
boolean isReserve = action.getType().equals(TYPE_RESERVE); boolean isReserve = action.getType().equals(TYPE_RESERVE);
boolean isRelease = action.getType().equals(TYPE_RELEASE); boolean isRelease = action.getType().equals(TYPE_RELEASE);