Apply dynamic changes in ThreadPoolTaskExecutor before setting local value

If the ThreadPoolTaskExecutor is dynamically changed with an invalid value
the state of the ThreadPoolTaskExecutor does no longer correctly represent
the state of the underlying ThreadPoolExecutor

Author: filiphr

spring-context/src/main/java/org/springframework/scheduling/concurrent/ThreadPoolTaskExecutor.java

@@ -112,10 +112,10 @@ public class ThreadPoolTaskExecutor extends ExecutorConfigurationSupport
 	 */
 	public void setCorePoolSize(int corePoolSize) {
 		synchronized (this.poolSizeMonitor) {
-			this.corePoolSize = corePoolSize;
 			if (this.threadPoolExecutor != null) {
 				this.threadPoolExecutor.setCorePoolSize(corePoolSize);
 			}
+			this.corePoolSize = corePoolSize;
 		}
 	}
 
@@ -135,10 +135,10 @@ public int getCorePoolSize() {
 	 */
 	public void setMaxPoolSize(int maxPoolSize) {
 		synchronized (this.poolSizeMonitor) {
-			this.maxPoolSize = maxPoolSize;
 			if (this.threadPoolExecutor != null) {
 				this.threadPoolExecutor.setMaximumPoolSize(maxPoolSize);
 			}
+			this.maxPoolSize = maxPoolSize;
 		}
 	}
 
@@ -158,10 +158,10 @@ public int getMaxPoolSize() {
 	 */
 	public void setKeepAliveSeconds(int keepAliveSeconds) {
 		synchronized (this.poolSizeMonitor) {
-			this.keepAliveSeconds = keepAliveSeconds;
 			if (this.threadPoolExecutor != null) {
 				this.threadPoolExecutor.setKeepAliveTime(keepAliveSeconds, TimeUnit.SECONDS);
 			}
+			this.keepAliveSeconds = keepAliveSeconds;
 		}
 	}
 

spring-context/src/test/java/org/springframework/scheduling/concurrent/ThreadPoolTaskExecutorTests.java

@@ -16,21 +16,96 @@
 
 package org.springframework.scheduling.concurrent;
 
+import java.util.concurrent.TimeUnit;
+
+import org.junit.jupiter.api.Test;
 import org.springframework.core.task.AsyncListenableTaskExecutor;
 
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
 /**
  * @author Juergen Hoeller
  * @since 5.0.5
  */
 class ThreadPoolTaskExecutorTests extends AbstractSchedulingTaskExecutorTests {
 
+	private final ThreadPoolTaskExecutor executor = new ThreadPoolTaskExecutor();
+
 	@Override
 	protected AsyncListenableTaskExecutor buildExecutor() {
-		ThreadPoolTaskExecutor executor = new ThreadPoolTaskExecutor();
 		executor.setThreadNamePrefix(this.threadNamePrefix);
 		executor.setMaxPoolSize(1);
 		executor.afterPropertiesSet();
 		return executor;
 	}
 
+	@Test
+	void modifyCorePoolSizeWhileRunning() {
+		assertThat(executor.getCorePoolSize()).isEqualTo(1);
+		assertThat(executor.getThreadPoolExecutor().getCorePoolSize()).isEqualTo(1);
+
+		executor.setCorePoolSize(0);
+
+		assertThat(executor.getCorePoolSize()).isEqualTo(0);
+		assertThat(executor.getThreadPoolExecutor().getCorePoolSize()).isEqualTo(0);
+	}
+
+	@Test
+	void modifyCorePoolSizeWithInvalidValueWhileRunning() {
+		assertThat(executor.getCorePoolSize()).isEqualTo(1);
+		assertThat(executor.getThreadPoolExecutor().getCorePoolSize()).isEqualTo(1);
+
+		assertThatThrownBy(() -> executor.setCorePoolSize(-1))
+				.isInstanceOf(IllegalArgumentException.class);
+
+		assertThat(executor.getCorePoolSize()).isEqualTo(1);
+		assertThat(executor.getThreadPoolExecutor().getCorePoolSize()).isEqualTo(1);
+	}
+
+	@Test
+	void modifyMaxPoolSizeWhileRunning() {
+		assertThat(executor.getMaxPoolSize()).isEqualTo(1);
+		assertThat(executor.getThreadPoolExecutor().getMaximumPoolSize()).isEqualTo(1);
+
+		executor.setMaxPoolSize(5);
+
+		assertThat(executor.getMaxPoolSize()).isEqualTo(5);
+		assertThat(executor.getThreadPoolExecutor().getMaximumPoolSize()).isEqualTo(5);
+	}
+
+	@Test
+	void modifyMaxPoolSizeWithInvalidValueWhileRunning() {
+		assertThat(executor.getMaxPoolSize()).isEqualTo(1);
+		assertThat(executor.getThreadPoolExecutor().getMaximumPoolSize()).isEqualTo(1);
+
+		assertThatThrownBy(() -> executor.setMaxPoolSize(0))
+				.isInstanceOf(IllegalArgumentException.class);
+
+		assertThat(executor.getMaxPoolSize()).isEqualTo(1);
+		assertThat(executor.getThreadPoolExecutor().getMaximumPoolSize()).isEqualTo(1);
+	}
+
+	@Test
+	void modifyKeepAliveSecondsWhileRunning() {
+		assertThat(executor.getKeepAliveSeconds()).isEqualTo(60);
+		assertThat(executor.getThreadPoolExecutor().getKeepAliveTime(TimeUnit.SECONDS)).isEqualTo(60);
+
+		executor.setKeepAliveSeconds(10);
+
+		assertThat(executor.getKeepAliveSeconds()).isEqualTo(10);
+		assertThat(executor.getThreadPoolExecutor().getKeepAliveTime(TimeUnit.SECONDS)).isEqualTo(10);
+	}
+
+	@Test
+	void modifyKeepAliveSecondsWithInvalidValueWhileRunning() {
+		assertThat(executor.getKeepAliveSeconds()).isEqualTo(60);
+		assertThat(executor.getThreadPoolExecutor().getKeepAliveTime(TimeUnit.SECONDS)).isEqualTo(60);
+
+		assertThatThrownBy(() -> executor.setKeepAliveSeconds(-10))
+				.isInstanceOf(IllegalArgumentException.class);
+
+		assertThat(executor.getKeepAliveSeconds()).isEqualTo(60);
+		assertThat(executor.getThreadPoolExecutor().getKeepAliveTime(TimeUnit.SECONDS)).isEqualTo(60);
+	}
 }