Fix possible deadlock

- Removing some sync blocks which imho
  are not needed anymore.
- Adding some smoke tests.
- Relates to #364
This commit is contained in:
Janne Valkealahti
2017-06-10 09:11:35 +01:00
parent 78e3e479a7
commit bbca0ea504
3 changed files with 128 additions and 25 deletions

View File

@@ -115,9 +115,6 @@ 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;
/**
@@ -177,12 +174,10 @@ public abstract class AbstractStateMachine<S, E> extends StateMachineObjectSuppo
public State<S,E> getState() {
// if we're complete assume we're stopped
// and state was stashed into lastState
synchronized (lock) {
if (lastState != null && isComplete()) {
return lastState;
} else {
return currentState;
}
if (lastState != null && isComplete()) {
return lastState;
} else {
return currentState;
}
}
@@ -212,9 +207,7 @@ public abstract class AbstractStateMachine<S, E> extends StateMachineObjectSuppo
@Override
public boolean sendEvent(Message<E> event) {
synchronized (lock2) {
return sendEventInternal(event);
}
return sendEventInternal(event);
}
@Override
@@ -381,15 +374,13 @@ public abstract class AbstractStateMachine<S, E> extends StateMachineObjectSuppo
@Override
protected void doStop() {
synchronized (lock) {
stateMachineExecutor.stop();
notifyStateMachineStopped(buildStateContext(Stage.STATEMACHINE_STOP, null, null, this));
// stash current state before we null it so that
// we can still return where we 'were' when machine is stopped
lastState = currentState;
currentState = null;
initialEnabled = null;
}
stateMachineExecutor.stop();
notifyStateMachineStopped(buildStateContext(Stage.STATEMACHINE_STOP, null, null, this));
// stash current state before we null it so that
// we can still return where we 'were' when machine is stopped
lastState = currentState;
currentState = null;
initialEnabled = null;
}
@Override
@@ -938,9 +929,7 @@ public abstract class AbstractStateMachine<S, E> extends StateMachineObjectSuppo
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);
}
setCurrentStateInternal(state, message, transition, exit, stateMachine, sources, targets);
}
private void setCurrentStateInternal(State<S, E> state, Message<E> message, Transition<S, E> transition, boolean exit,

View File

@@ -0,0 +1,95 @@
/*
* Copyright 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.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
package org.springframework.statemachine;
import static org.hamcrest.CoreMatchers.nullValue;
import static org.hamcrest.Matchers.notNullValue;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.assertTrue;
import org.junit.Test;
import org.springframework.context.annotation.AnnotationConfigApplicationContext;
import org.springframework.context.annotation.Configuration;
import org.springframework.statemachine.config.EnableStateMachine;
import org.springframework.statemachine.config.EnumStateMachineConfigurerAdapter;
import org.springframework.statemachine.config.builders.StateMachineStateConfigurer;
import org.springframework.statemachine.config.builders.StateMachineTransitionConfigurer;
public class StateMachineSmokeTests extends AbstractStateMachineTests {
@Override
protected AnnotationConfigApplicationContext buildContext() {
return new AnnotationConfigApplicationContext();
}
@Test(timeout = 10000)
public void testLifecycleThreading() throws Exception {
context.register(Config1.class);
context.refresh();
assertTrue(context.containsBean(StateMachineSystemConstants.DEFAULT_ID_STATEMACHINE));
@SuppressWarnings("unchecked")
ObjectStateMachine<TestStates,TestEvents> machine =
context.getBean(StateMachineSystemConstants.DEFAULT_ID_STATEMACHINE, ObjectStateMachine.class);
assertThat(machine, notNullValue());
int cnt = 10;
Thread[] threads = new Thread[cnt];
for (int i = 0; i < cnt; i++) {
threads[i] = new Thread(() -> {
for (int j = 0; j < 1000; j++) {
machine.start();
machine.sendEvent(TestEvents.E1);
machine.stop();
}
});
}
for (Thread t : threads) {
t.start();
}
for (Thread t : threads) {
t.join();
}
assertThat(machine.getState(), nullValue());
}
@Configuration
@EnableStateMachine
static class Config1 extends EnumStateMachineConfigurerAdapter<TestStates, TestEvents> {
@Override
public void configure(StateMachineStateConfigurer<TestStates, TestEvents> states) throws Exception {
states
.withStates()
.initial(TestStates.S1)
.state(TestStates.S2);
}
@Override
public void configure(StateMachineTransitionConfigurer<TestStates, TestEvents> transitions) throws Exception {
transitions
.withExternal()
.source(TestStates.S1)
.target(TestStates.S2)
.event(TestEvents.E1)
.and()
.withExternal()
.source(TestStates.S2)
.target(TestStates.S1)
.event(TestEvents.E1);
}
}
}

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2015 the original author or authors.
* Copyright 2015-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.
@@ -89,6 +89,25 @@ public class PersistTests {
assertThat(persist.listDbEntries(), containsString("id=2, state=SENT"));
}
@Test
public void testThreadingSmoke() throws Exception {
int cnt = 10;
Thread[] threads = new Thread[cnt];
for (int i = 0; i < cnt; i++) {
threads[i] = new Thread(() -> {
for (int j = 0; j < 1000; j++) {
persist.change(3, "SEND");
}
});
}
for (Thread t : threads) {
t.start();
}
for (Thread t : threads) {
t.join();
}
}
private static class TestListener extends StateMachineListenerAdapter<String, String> {
volatile CountDownLatch stateChangedLatch = new CountDownLatch(1);