From 7fc7dd9caca55d818b33f5350c9b44453cb5312b Mon Sep 17 00:00:00 2001 From: Janne Valkealahti Date: Sat, 8 Aug 2015 16:30:24 +0100 Subject: [PATCH] Fix potential concurrency issue with join pseudostate - Change notified flag to volatile which might explain some test failures. - More testing tweaks for #76 --- .../statemachine/state/JoinPseudoState.java | 2 +- .../src/test/java/demo/tasks/TasksTests.java | 6 +++--- .../ZookeeperStateMachineEnsembleTests.java | 19 +++++++++++++++---- 3 files changed, 19 insertions(+), 8 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 13f1f6ba..79a0940d 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 @@ -73,7 +73,7 @@ public class JoinPseudoState extends AbstractPseudoState { private final PseudoState pseudoState; private final List> track; - private boolean notified = false; + private volatile boolean notified = false; public JoinTracker(PseudoState pseudoState, List> track) { this.pseudoState = pseudoState; 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 d13d6c9b..15741697 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 @@ -63,7 +63,7 @@ public class TasksTests { public void testRunOnce() throws InterruptedException { listener.reset(9, 0, 0); tasks.run(); - assertThat(listener.stateChangedLatch.await(6, TimeUnit.SECONDS), is(true)); + assertThat(listener.stateChangedLatch.await(8, TimeUnit.SECONDS), is(true)); assertThat(machine.getState().getIds(), contains(States.READY)); Map variables = machine.getExtendedState().getVariables(); assertThat(variables.size(), is(3)); @@ -73,7 +73,7 @@ public class TasksTests { public void testRunTwice() throws InterruptedException { listener.reset(9, 0, 0); tasks.run(); - assertThat(listener.stateChangedLatch.await(6, TimeUnit.SECONDS), is(true)); + assertThat(listener.stateChangedLatch.await(8, TimeUnit.SECONDS), is(true)); assertThat(machine.getState().getIds(), contains(States.READY)); Map variables = machine.getExtendedState().getVariables(); @@ -81,7 +81,7 @@ public class TasksTests { listener.reset(9, 0, 0); tasks.run(); - assertThat(listener.stateChangedLatch.await(6, TimeUnit.SECONDS), is(true)); + assertThat(listener.stateChangedLatch.await(8, TimeUnit.SECONDS), is(true)); assertThat(machine.getState().getIds(), contains(States.READY)); variables = machine.getExtendedState().getVariables(); diff --git a/spring-statemachine-zookeeper/src/test/java/org/springframework/statemachine/zookeeper/ZookeeperStateMachineEnsembleTests.java b/spring-statemachine-zookeeper/src/test/java/org/springframework/statemachine/zookeeper/ZookeeperStateMachineEnsembleTests.java index ce408d3c..c901d860 100644 --- a/spring-statemachine-zookeeper/src/test/java/org/springframework/statemachine/zookeeper/ZookeeperStateMachineEnsembleTests.java +++ b/spring-statemachine-zookeeper/src/test/java/org/springframework/statemachine/zookeeper/ZookeeperStateMachineEnsembleTests.java @@ -16,8 +16,8 @@ package org.springframework.statemachine.zookeeper; import static org.hamcrest.Matchers.greaterThan; -import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.instanceOf; +import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.nullValue; import static org.junit.Assert.assertThat; @@ -551,16 +551,27 @@ public class ZookeeperStateMachineEnsembleTests extends AbstractZookeeperTests { ensemble.afterPropertiesSet(); ensemble.start(); listener.reset(0, 10, 1); - ensemble.enabled = false; - for (int i = 0; i < 5; i++) { + // this is a bit of a hack to test things like this + // not sure if this is totally reliable way + ensemble.enabled = false; + for (int i = 0; i < 4; i++) { + ensemble.setState(new DefaultStateMachineContext("S" + i, "E" + i, + new HashMap(), new DefaultExtendedState())); + } + assertThat(listener.errorLatch.await(2, TimeUnit.SECONDS), is(false)); + for (int i = 4; i < 5; i++) { ensemble.setState(new DefaultStateMachineContext("S" + i, "E" + i, new HashMap(), new DefaultExtendedState())); } ensemble.enabled = true; TestUtils.callMethod("registerWatcherForStatePath", ensemble); - assertThat(listener.errors.size(), is(0)); + String reason = ""; + if (listener.errors.size() > 0) { + reason = listener.errors.get(0).toString(); + } + assertThat(reason, listener.errors.size(), is(0)); for (int i = 5; i < 6; i++) { ensemble.setState(new DefaultStateMachineContext("S" + i, "E" + i,