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

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,6 +16,12 @@
 
 package org.springframework.scheduling.concurrent;
 
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import java.util.concurrent.TimeUnit;
+
+import org.junit.jupiter.api.Test;
 import org.springframework.core.task.AsyncListenableTaskExecutor;
 
 /**
@@ -24,13 +30,82 @@
  */
 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);
+	}
 }