From 24446c9b0a9dab04ac326f4c138b99580f842944 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Mon, 26 Jun 2023 19:28:08 +0200 Subject: [PATCH] Avoid arithmetic overflow for large delay/period values Closes gh-30754 --- .../concurrent/ConcurrentTaskScheduler.java | 18 +++++++---- .../concurrent/ThreadPoolTaskScheduler.java | 18 +++++++---- ...duledAnnotationBeanPostProcessorTests.java | 30 +++++++++++-------- 3 files changed, 43 insertions(+), 23 deletions(-) diff --git a/spring-context/src/main/java/org/springframework/scheduling/concurrent/ConcurrentTaskScheduler.java b/spring-context/src/main/java/org/springframework/scheduling/concurrent/ConcurrentTaskScheduler.java index 2e72a880a719..713ed1513866 100644 --- a/spring-context/src/main/java/org/springframework/scheduling/concurrent/ConcurrentTaskScheduler.java +++ b/spring-context/src/main/java/org/springframework/scheduling/concurrent/ConcurrentTaskScheduler.java @@ -70,6 +70,9 @@ */ public class ConcurrentTaskScheduler extends ConcurrentTaskExecutor implements TaskScheduler { + private static final TimeUnit NANO = TimeUnit.NANOSECONDS; + + @Nullable private static Class managedScheduledExecutorServiceClass; @@ -211,7 +214,8 @@ public ScheduledFuture schedule(Runnable task, Trigger trigger) { public ScheduledFuture schedule(Runnable task, Instant startTime) { Duration initialDelay = Duration.between(this.clock.instant(), startTime); try { - return this.scheduledExecutor.schedule(decorateTask(task, false), initialDelay.toNanos(), TimeUnit.NANOSECONDS); + return this.scheduledExecutor.schedule(decorateTask(task, false), + NANO.convert(initialDelay), NANO); } catch (RejectedExecutionException ex) { throw new TaskRejectedException("Executor [" + this.scheduledExecutor + "] did not accept task: " + task, ex); @@ -222,7 +226,8 @@ public ScheduledFuture schedule(Runnable task, Instant startTime) { public ScheduledFuture scheduleAtFixedRate(Runnable task, Instant startTime, Duration period) { Duration initialDelay = Duration.between(this.clock.instant(), startTime); try { - return this.scheduledExecutor.scheduleAtFixedRate(decorateTask(task, true), initialDelay.toNanos(), period.toNanos(), TimeUnit.NANOSECONDS); + return this.scheduledExecutor.scheduleAtFixedRate(decorateTask(task, true), + NANO.convert(initialDelay), NANO.convert(period), NANO); } catch (RejectedExecutionException ex) { throw new TaskRejectedException("Executor [" + this.scheduledExecutor + "] did not accept task: " + task, ex); @@ -232,7 +237,8 @@ public ScheduledFuture scheduleAtFixedRate(Runnable task, Instant startTime, @Override public ScheduledFuture scheduleAtFixedRate(Runnable task, Duration period) { try { - return this.scheduledExecutor.scheduleAtFixedRate(decorateTask(task, true), 0, period.toNanos(), TimeUnit.NANOSECONDS); + return this.scheduledExecutor.scheduleAtFixedRate(decorateTask(task, true), + 0, NANO.convert(period), NANO); } catch (RejectedExecutionException ex) { throw new TaskRejectedException("Executor [" + this.scheduledExecutor + "] did not accept task: " + task, ex); @@ -243,7 +249,8 @@ public ScheduledFuture scheduleAtFixedRate(Runnable task, Duration period) { public ScheduledFuture scheduleWithFixedDelay(Runnable task, Instant startTime, Duration delay) { Duration initialDelay = Duration.between(this.clock.instant(), startTime); try { - return this.scheduledExecutor.scheduleWithFixedDelay(decorateTask(task, true), initialDelay.toNanos(), delay.toNanos(), TimeUnit.NANOSECONDS); + return this.scheduledExecutor.scheduleWithFixedDelay(decorateTask(task, true), + NANO.convert(initialDelay), NANO.convert(delay), NANO); } catch (RejectedExecutionException ex) { throw new TaskRejectedException("Executor [" + this.scheduledExecutor + "] did not accept task: " + task, ex); @@ -253,7 +260,8 @@ public ScheduledFuture scheduleWithFixedDelay(Runnable task, Instant startTim @Override public ScheduledFuture scheduleWithFixedDelay(Runnable task, Duration delay) { try { - return this.scheduledExecutor.scheduleWithFixedDelay(decorateTask(task, true), 0, delay.toNanos(), TimeUnit.NANOSECONDS); + return this.scheduledExecutor.scheduleWithFixedDelay(decorateTask(task, true), + 0, NANO.convert(delay), NANO); } catch (RejectedExecutionException ex) { throw new TaskRejectedException("Executor [" + this.scheduledExecutor + "] did not accept task: " + task, ex); diff --git a/spring-context/src/main/java/org/springframework/scheduling/concurrent/ThreadPoolTaskScheduler.java b/spring-context/src/main/java/org/springframework/scheduling/concurrent/ThreadPoolTaskScheduler.java index 57891e24e7ef..7a6c8b8e35a1 100644 --- a/spring-context/src/main/java/org/springframework/scheduling/concurrent/ThreadPoolTaskScheduler.java +++ b/spring-context/src/main/java/org/springframework/scheduling/concurrent/ThreadPoolTaskScheduler.java @@ -63,6 +63,9 @@ public class ThreadPoolTaskScheduler extends ExecutorConfigurationSupport implements AsyncListenableTaskExecutor, SchedulingTaskExecutor, TaskScheduler { + private static final TimeUnit NANO = TimeUnit.NANOSECONDS; + + private volatile int poolSize = 1; private volatile boolean removeOnCancelPolicy; @@ -376,7 +379,8 @@ public ScheduledFuture schedule(Runnable task, Instant startTime) { ScheduledExecutorService executor = getScheduledExecutor(); Duration initialDelay = Duration.between(this.clock.instant(), startTime); try { - return executor.schedule(errorHandlingTask(task, false), initialDelay.toNanos(), TimeUnit.NANOSECONDS); + return executor.schedule(errorHandlingTask(task, false), + NANO.convert(initialDelay), NANO); } catch (RejectedExecutionException ex) { throw new TaskRejectedException("Executor [" + executor + "] did not accept task: " + task, ex); @@ -388,7 +392,8 @@ public ScheduledFuture scheduleAtFixedRate(Runnable task, Instant startTime, ScheduledExecutorService executor = getScheduledExecutor(); Duration initialDelay = Duration.between(this.clock.instant(), startTime); try { - return executor.scheduleAtFixedRate(errorHandlingTask(task, true), initialDelay.toNanos(), period.toNanos(), TimeUnit.NANOSECONDS); + return executor.scheduleAtFixedRate(errorHandlingTask(task, true), + NANO.convert(initialDelay), NANO.convert(period), NANO); } catch (RejectedExecutionException ex) { throw new TaskRejectedException("Executor [" + executor + "] did not accept task: " + task, ex); @@ -399,7 +404,8 @@ public ScheduledFuture scheduleAtFixedRate(Runnable task, Instant startTime, public ScheduledFuture scheduleAtFixedRate(Runnable task, Duration period) { ScheduledExecutorService executor = getScheduledExecutor(); try { - return executor.scheduleAtFixedRate(errorHandlingTask(task, true), 0, period.toNanos(), TimeUnit.NANOSECONDS); + return executor.scheduleAtFixedRate(errorHandlingTask(task, true), + 0, NANO.convert(period), NANO); } catch (RejectedExecutionException ex) { throw new TaskRejectedException("Executor [" + executor + "] did not accept task: " + task, ex); @@ -411,7 +417,8 @@ public ScheduledFuture scheduleWithFixedDelay(Runnable task, Instant startTim ScheduledExecutorService executor = getScheduledExecutor(); Duration initialDelay = Duration.between(this.clock.instant(), startTime); try { - return executor.scheduleWithFixedDelay(errorHandlingTask(task, true), initialDelay.toNanos(), delay.toNanos(), TimeUnit.NANOSECONDS); + return executor.scheduleWithFixedDelay(errorHandlingTask(task, true), + NANO.convert(initialDelay), NANO.convert(delay), NANO); } catch (RejectedExecutionException ex) { throw new TaskRejectedException("Executor [" + executor + "] did not accept task: " + task, ex); @@ -422,7 +429,8 @@ public ScheduledFuture scheduleWithFixedDelay(Runnable task, Instant startTim public ScheduledFuture scheduleWithFixedDelay(Runnable task, Duration delay) { ScheduledExecutorService executor = getScheduledExecutor(); try { - return executor.scheduleWithFixedDelay(errorHandlingTask(task, true), 0, delay.toNanos(), TimeUnit.NANOSECONDS); + return executor.scheduleWithFixedDelay(errorHandlingTask(task, true), + 0, NANO.convert(delay), NANO); } catch (RejectedExecutionException ex) { throw new TaskRejectedException("Executor [" + executor + "] did not accept task: " + task, ex); diff --git a/spring-context/src/test/java/org/springframework/scheduling/annotation/ScheduledAnnotationBeanPostProcessorTests.java b/spring-context/src/test/java/org/springframework/scheduling/annotation/ScheduledAnnotationBeanPostProcessorTests.java index a1cd409d205f..417de9d8255c 100644 --- a/spring-context/src/test/java/org/springframework/scheduling/annotation/ScheduledAnnotationBeanPostProcessorTests.java +++ b/spring-context/src/test/java/org/springframework/scheduling/annotation/ScheduledAnnotationBeanPostProcessorTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2023 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. @@ -95,6 +95,7 @@ void closeContextAfterTest() { FixedDelay, 5_000 FixedDelayInSeconds, 5_000 FixedDelayInMinutes, 180_000 + FixedDelayWithMaxValue, -1 """) void fixedDelayTask(@NameToClass Class beanClass, long expectedInterval) { BeanDefinition processorDefinition = new RootBeanDefinition(ScheduledAnnotationBeanPostProcessor.class); @@ -120,7 +121,8 @@ void fixedDelayTask(@NameToClass Class beanClass, long expectedInterval) { assertThat(targetObject).isEqualTo(target); assertThat(targetMethod.getName()).isEqualTo("fixedDelay"); assertThat(task.getInitialDelayDuration()).isZero(); - assertThat(task.getIntervalDuration()).isEqualTo(Duration.ofMillis(expectedInterval)); + assertThat(task.getIntervalDuration()).isEqualTo( + Duration.ofMillis(expectedInterval < 0 ? Long.MAX_VALUE : expectedInterval)); } @ParameterizedTest @@ -343,8 +345,7 @@ void cronTaskWithInvalidZone() { BeanDefinition targetDefinition = new RootBeanDefinition(CronWithInvalidTimezoneTestBean.class); context.registerBeanDefinition("postProcessor", processorDefinition); context.registerBeanDefinition("target", targetDefinition); - assertThatExceptionOfType(BeanCreationException.class).isThrownBy( - context::refresh); + assertThatExceptionOfType(BeanCreationException.class).isThrownBy(context::refresh); } @Test @@ -355,8 +356,7 @@ void cronTaskWithMethodValidation() { context.registerBeanDefinition("methodValidation", validationDefinition); context.registerBeanDefinition("postProcessor", processorDefinition); context.registerBeanDefinition("target", targetDefinition); - assertThatExceptionOfType(BeanCreationException.class).isThrownBy( - context::refresh); + assertThatExceptionOfType(BeanCreationException.class).isThrownBy(context::refresh); } @Test @@ -702,18 +702,16 @@ void emptyAnnotation() { BeanDefinition targetDefinition = new RootBeanDefinition(EmptyAnnotationTestBean.class); context.registerBeanDefinition("postProcessor", processorDefinition); context.registerBeanDefinition("target", targetDefinition); - assertThatExceptionOfType(BeanCreationException.class).isThrownBy( - context::refresh); + assertThatExceptionOfType(BeanCreationException.class).isThrownBy(context::refresh); } @Test - void invalidCron() throws Throwable { + void invalidCron() { BeanDefinition processorDefinition = new RootBeanDefinition(ScheduledAnnotationBeanPostProcessor.class); BeanDefinition targetDefinition = new RootBeanDefinition(InvalidCronTestBean.class); context.registerBeanDefinition("postProcessor", processorDefinition); context.registerBeanDefinition("target", targetDefinition); - assertThatExceptionOfType(BeanCreationException.class).isThrownBy( - context::refresh); + assertThatExceptionOfType(BeanCreationException.class).isThrownBy(context::refresh); } @Test @@ -722,8 +720,7 @@ void nonEmptyParamList() { BeanDefinition targetDefinition = new RootBeanDefinition(NonEmptyParamListTestBean.class); context.registerBeanDefinition("postProcessor", processorDefinition); context.registerBeanDefinition("target", targetDefinition); - assertThatExceptionOfType(BeanCreationException.class).isThrownBy( - context::refresh); + assertThatExceptionOfType(BeanCreationException.class).isThrownBy(context::refresh); } @@ -748,6 +745,13 @@ void fixedDelay() { } } + static class FixedDelayWithMaxValue { + + @Scheduled(fixedDelay = Long.MAX_VALUE) + void fixedDelay() { + } + } + static class FixedRate {