From 98fbc94e93db609e12dd3ecedb07eaba7a025e79 Mon Sep 17 00:00:00 2001 From: Renan Freitas Date: Sun, 21 Aug 2022 15:24:51 +0200 Subject: [PATCH 1/2] Removes usage of ConcurrentTaskScheduler ConcurrentTaskScheduler doesn't expose any way for it's inner executor to be shutdown, when it's instance is created with the default constructor. This can lead to a scenario where a library user ends up with many unused threads with no clear cues about where they came from. This commit replaces the usage of ConcurrentTaskScheduler with ThreadPoolTaskScheduler, a type of similar semantics but better API, which includes methods for managing its internal resources lifecycle. Also it implements spring-beans interfaces which makes it more suitable to be managed by a Spring application context. - Fixes #624 --- .../StateMachineSystemConstants.java | 3 ++ .../config/StateMachineBuilder.java | 35 +++++++++++++++++-- .../StateMachineCommonConfiguration.java | 4 +-- 3 files changed, 37 insertions(+), 5 deletions(-) diff --git a/spring-statemachine-core/src/main/java/org/springframework/statemachine/StateMachineSystemConstants.java b/spring-statemachine-core/src/main/java/org/springframework/statemachine/StateMachineSystemConstants.java index c812a624e..e437f7741 100644 --- a/spring-statemachine-core/src/main/java/org/springframework/statemachine/StateMachineSystemConstants.java +++ b/spring-statemachine-core/src/main/java/org/springframework/statemachine/StateMachineSystemConstants.java @@ -38,4 +38,7 @@ public abstract class StateMachineSystemConstants { /** Bean name for task executor */ public static final String TASK_EXECUTOR_BEAN_NAME = "stateMachineTaskExecutor"; + /** Task scheduler threads prefix **/ + public static final String TASK_SCHEDULER_THREAD_PREFIX = "spring-state-machine-task-scheduler-"; + } diff --git a/spring-statemachine-core/src/main/java/org/springframework/statemachine/config/StateMachineBuilder.java b/spring-statemachine-core/src/main/java/org/springframework/statemachine/config/StateMachineBuilder.java index 258c055b9..3fded9c1c 100644 --- a/spring-statemachine-core/src/main/java/org/springframework/statemachine/config/StateMachineBuilder.java +++ b/spring-statemachine-core/src/main/java/org/springframework/statemachine/config/StateMachineBuilder.java @@ -16,9 +16,10 @@ package org.springframework.statemachine.config; import org.springframework.core.task.SyncTaskExecutor; -import org.springframework.scheduling.concurrent.ConcurrentTaskScheduler; +import org.springframework.scheduling.concurrent.ThreadPoolTaskScheduler; import org.springframework.statemachine.StateMachine; import org.springframework.statemachine.StateMachineException; +import org.springframework.statemachine.StateMachineSystemConstants; import org.springframework.statemachine.config.builders.StateMachineConfigBuilder; import org.springframework.statemachine.config.builders.StateMachineConfigurationBuilder; import org.springframework.statemachine.config.builders.StateMachineConfigurationConfigurer; @@ -35,6 +36,11 @@ import org.springframework.statemachine.config.model.ConfigurationData; import org.springframework.statemachine.config.model.StatesData; import org.springframework.statemachine.config.model.TransitionsData; +import org.springframework.statemachine.listener.StateMachineListener; +import org.springframework.statemachine.listener.StateMachineListenerAdapter; + +import java.util.ArrayList; +import java.util.Collection; /** * {@code StateMachineBuilder} provides a builder pattern for @@ -147,12 +153,35 @@ public StateMachine build() { } else { stateMachineFactory.setTaskExecutor(new SyncTaskExecutor()); } + + final Collection> resourceManagementListeners = new ArrayList<>(); if (stateMachineConfigurationConfig.getTaskScheduler() != null) { stateMachineFactory.setTaskScheduler(stateMachineConfigurationConfig.getTaskScheduler()); } else { - stateMachineFactory.setTaskScheduler(new ConcurrentTaskScheduler()); + final ThreadPoolTaskScheduler taskScheduler = new ThreadPoolTaskScheduler(); + taskScheduler.setThreadNamePrefix(StateMachineSystemConstants.TASK_SCHEDULER_THREAD_PREFIX); + taskScheduler.setPoolSize(1); + taskScheduler.setWaitForTasksToCompleteOnShutdown(true); + taskScheduler.setAwaitTerminationSeconds(10); + taskScheduler.afterPropertiesSet(); + stateMachineFactory.setTaskScheduler(taskScheduler); + resourceManagementListeners.add(new StateMachineListenerAdapter() { + @Override + public void stateMachineStarted(final StateMachine stateMachine) { + taskScheduler.afterPropertiesSet(); + } + + @Override + public void stateMachineStopped(final StateMachine stateMachine) { + taskScheduler.destroy(); + } + }); + } + final StateMachine stateMachine = stateMachineFactory.getStateMachine(); + for (final StateMachineListener listener : resourceManagementListeners) { + stateMachine.addStateListener(listener); } - return stateMachineFactory.getStateMachine(); + return stateMachine; } catch (Exception e) { throw new StateMachineException("Error building state machine", e); } diff --git a/spring-statemachine-core/src/main/java/org/springframework/statemachine/config/configuration/StateMachineCommonConfiguration.java b/spring-statemachine-core/src/main/java/org/springframework/statemachine/config/configuration/StateMachineCommonConfiguration.java index f3c5a86a0..e1391bd2c 100644 --- a/spring-statemachine-core/src/main/java/org/springframework/statemachine/config/configuration/StateMachineCommonConfiguration.java +++ b/spring-statemachine-core/src/main/java/org/springframework/statemachine/config/configuration/StateMachineCommonConfiguration.java @@ -20,7 +20,7 @@ import org.springframework.core.task.SyncTaskExecutor; import org.springframework.core.task.TaskExecutor; import org.springframework.scheduling.TaskScheduler; -import org.springframework.scheduling.concurrent.ConcurrentTaskScheduler; +import org.springframework.scheduling.concurrent.ThreadPoolTaskScheduler; import org.springframework.statemachine.StateMachineSystemConstants; /** @@ -39,7 +39,7 @@ public TaskExecutor taskExecutor() { @Bean public TaskScheduler taskScheduler() { - return new ConcurrentTaskScheduler(); + return new ThreadPoolTaskScheduler(); } @Bean(name = StateMachineHandlerApplicationListener.BEAN_NAME) From e2a811ca68b6e2da68ed390c4d7a1c07125fe232 Mon Sep 17 00:00:00 2001 From: Renan Freitas Date: Sun, 21 Aug 2022 15:40:46 +0200 Subject: [PATCH 2/2] Fixes TimerSmokeTests Fix a test that sets a TimerTrigger that may try to schedule a notification runnable after the internal TaskScheduler is already shutdown. See https://github.com/spring-projects/spring-statemachine/blob/2.5.1/spring-statemachine-core/src/main/java/org/springframework/statemachine/trigger/TimerTrigger.java#L117 --- .../statemachine/buildtests/TimerSmokeTests.java | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/spring-statemachine-build-tests/src/test/java/org/springframework/statemachine/buildtests/TimerSmokeTests.java b/spring-statemachine-build-tests/src/test/java/org/springframework/statemachine/buildtests/TimerSmokeTests.java index 3245e79f1..91bb7d489 100644 --- a/spring-statemachine-build-tests/src/test/java/org/springframework/statemachine/buildtests/TimerSmokeTests.java +++ b/spring-statemachine-build-tests/src/test/java/org/springframework/statemachine/buildtests/TimerSmokeTests.java @@ -16,7 +16,10 @@ package org.springframework.statemachine.buildtests; import org.junit.Test; +import org.springframework.core.task.SyncTaskExecutor; +import org.springframework.core.task.TaskExecutor; import org.springframework.scheduling.concurrent.ThreadPoolTaskExecutor; +import org.springframework.scheduling.concurrent.ThreadPoolTaskScheduler; import org.springframework.statemachine.StateMachine; import org.springframework.statemachine.config.StateMachineBuilder; import org.springframework.statemachine.test.StateMachineTestPlan; @@ -25,8 +28,10 @@ public class TimerSmokeTests { private static ThreadPoolTaskExecutor taskExecutor = new ThreadPoolTaskExecutor(); + private static ThreadPoolTaskScheduler taskScheduler = new ThreadPoolTaskScheduler(); { taskExecutor.initialize(); + taskScheduler.initialize(); } private StateMachine buildMachine() throws Exception { @@ -35,7 +40,8 @@ private StateMachine buildMachine() throws Exception { builder.configureConfiguration() .withConfiguration() - .taskExecutor(taskExecutor); + .taskExecutor(taskExecutor) + .taskScheduler(taskScheduler); builder.configureStates() .withStates()