Spring/spring-statemachine
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Add additinal sync

Browse Source 
- Change how some internals in
  AbstractStateMachine are synchronized
  to limit changes for deadlock.
- Relates #359
This commit is contained in:
Janne Valkealahti
2017-05-18 08:53:52 +01:00
parent 4d98b80358
commit ca02c2cecb
3 changed files with 96 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files

46
spring-statemachine-build-tests/src/test/java/org/springframework/statemachine/buildtests/TimerSmokeTests.java
View File

@@ -55,6 +55,37 @@ public class TimerSmokeTests {
return builder.build();
}
private StateMachine<String, String> buildMachine2() throws Exception {
StateMachineBuilder.Builder<String, String> builder = StateMachineBuilder.builder();
builder.configureConfiguration()
.withConfiguration()
.taskExecutor(taskExecutor);
builder.configureStates()
.withStates()
.initial("initial").end("end").and()
.withStates().parent("initial").initial("inner");
builder.configureTransitions()
.withExternal()
.source("initial")
.target("end")
.timerOnce(30)
.and()
.withExternal()
.source("inner")
.target("end")
.timerOnce(15)
.and()
.withLocal()
.source("inner")
.event("repeate");
return builder.build();
}
@Test
public void testNPE() throws Exception {
StateMachine<String, String> stateMachine;
@@ -67,6 +98,21 @@ public class TimerSmokeTests {
}
}
@Test
public void testNPE2() throws Exception {
StateMachine<String, String> stateMachine;
for (int i = 0; i < 20; i++) {
stateMachine = buildMachine2();
stateMachine.start();
while(!stateMachine.isComplete()) {
stateMachine.sendEvent("repeate");
}
stateMachine.stop();
}
}
@Test
public void testDeadlock() throws Exception {
StateMachineTestPlan<String, String> plan;

64
spring-statemachine-core/src/main/java/org/springframework/statemachine/support/AbstractStateMachine.java
View File

@@ -116,6 +116,7 @@ public abstract class AbstractStateMachine<S, E> extends StateMachineObjectSuppo
private volatile Message<E> forwardedInitialEvent;
private final Object lock = new Object();
private final Object lock2 = new Object();
private StateMachine<S, E> parentMachine;
@@ -211,30 +212,9 @@ public abstract class AbstractStateMachine<S, E> extends StateMachineObjectSuppo
@Override
public boolean sendEvent(Message<E> event) {
if (hasStateMachineError()) {
// TODO: should we throw exception?
notifyEventNotAccepted(buildStateContext(Stage.EVENT_NOT_ACCEPTED, event, null, getRelayStateMachine(), getState(), null));
return false;
synchronized (lock2) {
return sendEventInternal(event);
}
try {
event = getStateMachineInterceptors().preEvent(event, this);
} catch (Exception e) {
log.info("Event " + event + " threw exception in interceptors, not accepting event");
notifyEventNotAccepted(buildStateContext(Stage.EVENT_NOT_ACCEPTED, event, null, getRelayStateMachine(), getState(), null));
return false;
}
if (isComplete() || !isRunning()) {
notifyEventNotAccepted(buildStateContext(Stage.EVENT_NOT_ACCEPTED, event, null, getRelayStateMachine(), getState(), null));
return false;
}
boolean accepted = acceptEvent(event);
stateMachineExecutor.execute();
if (!accepted) {
notifyEventNotAccepted(buildStateContext(Stage.EVENT_NOT_ACCEPTED, event, null, getRelayStateMachine(), getState(), null));
}
return accepted;
}
@Override
@@ -555,6 +535,33 @@ public abstract class AbstractStateMachine<S, E> extends StateMachineObjectSuppo
forwardedInitialEvent = message;
}
private boolean sendEventInternal(Message<E> event) {
if (hasStateMachineError()) {
// TODO: should we throw exception?
notifyEventNotAccepted(buildStateContext(Stage.EVENT_NOT_ACCEPTED, event, null, getRelayStateMachine(), getState(), null));
return false;
}
try {
event = getStateMachineInterceptors().preEvent(event, this);
} catch (Exception e) {
log.info("Event " + event + " threw exception in interceptors, not accepting event");
notifyEventNotAccepted(buildStateContext(Stage.EVENT_NOT_ACCEPTED, event, null, getRelayStateMachine(), getState(), null));
return false;
}
if (isComplete() || !isRunning()) {
notifyEventNotAccepted(buildStateContext(Stage.EVENT_NOT_ACCEPTED, event, null, getRelayStateMachine(), getState(), null));
return false;
}
boolean accepted = acceptEvent(event);
stateMachineExecutor.execute();
if (!accepted) {
notifyEventNotAccepted(buildStateContext(Stage.EVENT_NOT_ACCEPTED, event, null, getRelayStateMachine(), getState(), null));
}
return accepted;
}
private StateMachine<S, E> getRelayStateMachine() {
return relay != null ? relay : this;
}
@@ -924,11 +931,18 @@ public abstract class AbstractStateMachine<S, E> extends StateMachineObjectSuppo
return null;
}
synchronized void setCurrentState(State<S, E> state, Message<E> message, Transition<S, E> transition, boolean exit, StateMachine<S, E> stateMachine) {
void setCurrentState(State<S, E> state, Message<E> message, Transition<S, E> transition, boolean exit, StateMachine<S, E> stateMachine) {
setCurrentState(state, message, transition, exit, stateMachine, null, null);
}
synchronized void setCurrentState(State<S, E> state, Message<E> message, Transition<S, E> transition, boolean exit,
void setCurrentState(State<S, E> state, Message<E> message, Transition<S, E> transition, boolean exit,
StateMachine<S, E> stateMachine, Collection<State<S, E>> sources, Collection<State<S, E>> targets) {
synchronized (lock2) {
setCurrentStateInternal(state, message, transition, exit, stateMachine, sources, targets);
}
}
private void setCurrentStateInternal(State<S, E> state, Message<E> message, Transition<S, E> transition, boolean exit,
StateMachine<S, E> stateMachine, Collection<State<S, E>> sources, Collection<State<S, E>> targets) {
State<S, E> findDeep = findDeepParent(state);
boolean isTargetSubOf = false;

12
spring-statemachine-core/src/test/java/org/springframework/statemachine/action/ActionAndTimerTests.java
View File

@@ -1,5 +1,5 @@
/*
* Copyright 2016 the original author or authors.
* Copyright 2016-2017 the original author or authors.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
@@ -59,6 +59,11 @@ public class ActionAndTimerTests extends AbstractStateMachineTests {
assertThat(testTimerAction.latch.await(4, TimeUnit.SECONDS), is(true));
assertThat(testTimerAction.e, nullValue());
// need to sleep for TimerTrigger not causing
// next event to get handled with threads, thus
// causing interrupt
Thread.sleep(1000);
machine.sendEvent(TestEvents.E2);
assertThat(testListener.s3EnteredLatch.await(2, TimeUnit.SECONDS), is(true));
assertThat(machine.getState().getIds(), containsInAnyOrder(TestStates.S3));
@@ -84,6 +89,11 @@ public class ActionAndTimerTests extends AbstractStateMachineTests {
assertThat(testTimerAction.latch.await(4, TimeUnit.SECONDS), is(true));
assertThat(testTimerAction.e, nullValue());
// need to sleep for TimerTrigger not causing
// next event to get handled with threads, thus
// causing interrupt
Thread.sleep(1000);
machine.sendEvent(TestEvents.E2);
assertThat(testListener.s3EnteredLatch.await(2, TimeUnit.SECONDS), is(true));
assertThat(machine.getState().getIds(), containsInAnyOrder(TestStates.S3));