From bba3b231b3daaeb7bd25cf3286cdb42cdcf920f1 Mon Sep 17 00:00:00 2001 From: Janne Valkealahti Date: Fri, 7 Aug 2015 14:38:17 +0100 Subject: [PATCH] Synchronize Join PseudoState state machine listener - Synchronize JoinPseudoState's JoinTracker.stateChanged(State, State) method for playing nice with concurrent region execution. Should fix #93. - Tuning test which relates to #76. What comes for the tests, there was this concurrency issue and also problem in tests itself. - Polish JoinPseudoState. --- .../statemachine/state/JoinPseudoState.java | 13 +++- .../recipes/TasksHandlerTests.java | 68 ++++++++++++------- .../src/test/java/demo/tasks/TasksTests.java | 44 +++++++----- 3 files changed, 83 insertions(+), 42 deletions(-) diff --git a/spring-statemachine-core/src/main/java/org/springframework/statemachine/state/JoinPseudoState.java b/spring-statemachine-core/src/main/java/org/springframework/statemachine/state/JoinPseudoState.java index 052dffa5..13f1f6ba 100644 --- a/spring-statemachine-core/src/main/java/org/springframework/statemachine/state/JoinPseudoState.java +++ b/spring-statemachine-core/src/main/java/org/springframework/statemachine/state/JoinPseudoState.java @@ -33,9 +33,13 @@ import org.springframework.statemachine.state.PseudoStateContext.PseudoAction; public class JoinPseudoState extends AbstractPseudoState { private final List> joins; - private volatile JoinTracker tracker; + /** + * Instantiates a new join pseudo state. + * + * @param joins the joins + */ public JoinPseudoState(List> joins) { super(PseudoStateKind.JOIN); this.joins = joins; @@ -56,6 +60,11 @@ public class JoinPseudoState extends AbstractPseudoState { tracker = null; } + /** + * Gets the join states. + * + * @return the joins + */ public List> getJoins() { return joins; } @@ -72,7 +81,7 @@ public class JoinPseudoState extends AbstractPseudoState { } @Override - public void stateChanged(State from, State to) { + public synchronized void stateChanged(State from, State to) { if (!notified && track.size() > 0) { track.remove(to); if (track.size() == 0) { diff --git a/spring-statemachine-recipes/src/test/java/org/springframework/statemachine/recipes/TasksHandlerTests.java b/spring-statemachine-recipes/src/test/java/org/springframework/statemachine/recipes/TasksHandlerTests.java index a3d88830..3844b32a 100644 --- a/spring-statemachine-recipes/src/test/java/org/springframework/statemachine/recipes/TasksHandlerTests.java +++ b/spring-statemachine-recipes/src/test/java/org/springframework/statemachine/recipes/TasksHandlerTests.java @@ -126,16 +126,19 @@ public class TasksHandlerTests { handler.addTasksListener(tasksListener); TestListener listener = new TestListener(); - listener.reset(12, 0, 0); + listener.reset(1, 0, 0); StateMachine machine = handler.getStateMachine(); machine.addStateListener(listener); machine.start(); assertThat(listener.stateMachineStartedLatch.await(1, TimeUnit.SECONDS), is(true)); + assertThat(listener.stateChangedLatch.await(2, TimeUnit.SECONDS), is(true)); + assertThat(listener.stateChangedCount, is(1)); + + listener.reset(11, 0, 0); handler.runTasks(); - - assertThat(listener.stateChangedLatch.await(8, TimeUnit.SECONDS), is(true)); - assertThat(listener.stateChangedCount, is(12)); + assertThat(listener.stateChangedLatch.await(4, TimeUnit.SECONDS), is(true)); + assertThat(listener.stateChangedCount, is(11)); assertThat(machine.getState().getIds(), contains(TasksHandler.STATE_READY)); } @@ -178,17 +181,20 @@ public class TasksHandlerTests { .build(); TestListener listener = new TestListener(); - listener.reset(22, 0, 0); StateMachine machine = handler.getStateMachine(); machine.addStateListener(listener); + listener.reset(1, 0, 0); machine.start(); assertThat(listener.stateMachineStartedLatch.await(1, TimeUnit.SECONDS), is(true)); + assertThat(listener.stateChangedLatch.await(2, TimeUnit.SECONDS), is(true)); + assertThat(listener.stateChangedCount, is(1)); + listener.reset(21, 0, 0); handler.runTasks(); - assertThat(listener.stateChangedLatch.await(20, TimeUnit.SECONDS), is(true)); - assertThat(listener.stateChangedCount, is(22)); + assertThat(listener.stateChangedLatch.await(10, TimeUnit.SECONDS), is(true)); + assertThat(listener.stateChangedCount, is(21)); assertThat(machine.getState().getIds(), contains(TasksHandler.STATE_READY)); Map variables = machine.getExtendedState().getVariables(); assertThat(variables.size(), is(9)); @@ -325,6 +331,8 @@ public class TasksHandlerTests { static class TestListener extends StateMachineListenerAdapter { + private final Object lock = new Object(); + volatile CountDownLatch stateMachineStartedLatch = new CountDownLatch(1); volatile CountDownLatch stateChangedLatch = new CountDownLatch(1); volatile CountDownLatch stateEnteredLatch = new CountDownLatch(2); @@ -337,31 +345,41 @@ public class TasksHandlerTests { @Override public void stateMachineStarted(StateMachine stateMachine) { - stateMachineStartedLatch.countDown(); + synchronized (lock) { + stateMachineStartedLatch.countDown(); + } } @Override public void stateChanged(State from, State to) { - stateChangedCount++; - stateChangedLatch.countDown(); + synchronized (lock) { + stateChangedCount++; + stateChangedLatch.countDown(); + } } @Override public void stateEntered(State state) { - statesEntered.add(state); - stateEnteredLatch.countDown(); + synchronized (lock) { + statesEntered.add(state); + stateEnteredLatch.countDown(); + } } @Override public void stateExited(State state) { - statesExited.add(state); - stateExitedLatch.countDown(); + synchronized (lock) { + statesExited.add(state); + stateExitedLatch.countDown(); + } } @Override public void transitionEnded(Transition transition) { - transitionCount++; - transitionLatch.countDown(); + synchronized (lock) { + transitionCount++; + transitionLatch.countDown(); + } } public void reset(int c1, int c2, int c3) { @@ -369,14 +387,16 @@ public class TasksHandlerTests { } public void reset(int c1, int c2, int c3, int c4) { - stateChangedLatch = new CountDownLatch(c1); - stateEnteredLatch = new CountDownLatch(c2); - stateExitedLatch = new CountDownLatch(c3); - transitionLatch = new CountDownLatch(c4); - stateChangedCount = 0; - transitionCount = 0; - statesEntered.clear(); - statesExited.clear(); + synchronized (lock) { + stateChangedLatch = new CountDownLatch(c1); + stateEnteredLatch = new CountDownLatch(c2); + stateExitedLatch = new CountDownLatch(c3); + transitionLatch = new CountDownLatch(c4); + stateChangedCount = 0; + transitionCount = 0; + statesEntered.clear(); + statesExited.clear(); + } } } diff --git a/spring-statemachine-samples/tasks/src/test/java/demo/tasks/TasksTests.java b/spring-statemachine-samples/tasks/src/test/java/demo/tasks/TasksTests.java index e5255c86..d13d6c9b 100644 --- a/spring-statemachine-samples/tasks/src/test/java/demo/tasks/TasksTests.java +++ b/spring-statemachine-samples/tasks/src/test/java/demo/tasks/TasksTests.java @@ -156,6 +156,8 @@ public class TasksTests { static class TestListener extends StateMachineListenerAdapter { + final Object lock = new Object(); + volatile CountDownLatch stateChangedLatch = new CountDownLatch(1); volatile CountDownLatch stateEnteredLatch = new CountDownLatch(2); volatile CountDownLatch stateExitedLatch = new CountDownLatch(0); @@ -167,26 +169,34 @@ public class TasksTests { @Override public void stateChanged(State from, State to) { - stateChangedCount++; - stateChangedLatch.countDown(); + synchronized (lock) { + stateChangedCount++; + stateChangedLatch.countDown(); + } } @Override public void stateEntered(State state) { - statesEntered.add(state); - stateEnteredLatch.countDown(); + synchronized (lock) { + statesEntered.add(state); + stateEnteredLatch.countDown(); + } } @Override public void stateExited(State state) { - statesExited.add(state); - stateExitedLatch.countDown(); + synchronized (lock) { + statesExited.add(state); + stateExitedLatch.countDown(); + } } @Override public void transitionEnded(Transition transition) { - transitionCount++; - transitionLatch.countDown(); + synchronized (lock) { + transitionCount++; + transitionLatch.countDown(); + } } public void reset(int c1, int c2, int c3) { @@ -194,14 +204,16 @@ public class TasksTests { } public void reset(int c1, int c2, int c3, int c4) { - stateChangedLatch = new CountDownLatch(c1); - stateEnteredLatch = new CountDownLatch(c2); - stateExitedLatch = new CountDownLatch(c3); - transitionLatch = new CountDownLatch(c4); - stateChangedCount = 0; - transitionCount = 0; - statesEntered.clear(); - statesExited.clear(); + synchronized (lock) { + stateChangedLatch = new CountDownLatch(c1); + stateEnteredLatch = new CountDownLatch(c2); + stateExitedLatch = new CountDownLatch(c3); + transitionLatch = new CountDownLatch(c4); + stateChangedCount = 0; + transitionCount = 0; + statesEntered.clear(); + statesExited.clear(); + } } }