Skip to content

Commit 599ac58

Browse files
jhoellerjhoeller
committed
Avoid arithmetic overflow for large delay/period values
Closes gh-30754
1 parent 449174c commit 599ac58

File tree

3 files changed

+43
-23
lines changed

3 files changed

+43
-23
lines changed

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

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,9 @@
7070
*/
7171
public class ConcurrentTaskScheduler extends ConcurrentTaskExecutor implements TaskScheduler {
7272

73+
private static final TimeUnit NANO = TimeUnit.NANOSECONDS;
74+
75+
7376
@Nullable
7477
private static Class<?> managedScheduledExecutorServiceClass;
7578

@@ -211,7 +214,8 @@ public ScheduledFuture<?> schedule(Runnable task, Trigger trigger) {
211214
public ScheduledFuture<?> schedule(Runnable task, Instant startTime) {
212215
Duration initialDelay = Duration.between(this.clock.instant(), startTime);
213216
try {
214-
return this.scheduledExecutor.schedule(decorateTask(task, false), initialDelay.toNanos(), TimeUnit.NANOSECONDS);
217+
return this.scheduledExecutor.schedule(decorateTask(task, false),
218+
NANO.convert(initialDelay), NANO);
215219
}
216220
catch (RejectedExecutionException ex) {
217221
throw new TaskRejectedException("Executor [" + this.scheduledExecutor + "] did not accept task: " + task, ex);
@@ -222,7 +226,8 @@ public ScheduledFuture<?> schedule(Runnable task, Instant startTime) {
222226
public ScheduledFuture<?> scheduleAtFixedRate(Runnable task, Instant startTime, Duration period) {
223227
Duration initialDelay = Duration.between(this.clock.instant(), startTime);
224228
try {
225-
return this.scheduledExecutor.scheduleAtFixedRate(decorateTask(task, true), initialDelay.toNanos(), period.toNanos(), TimeUnit.NANOSECONDS);
229+
return this.scheduledExecutor.scheduleAtFixedRate(decorateTask(task, true),
230+
NANO.convert(initialDelay), NANO.convert(period), NANO);
226231
}
227232
catch (RejectedExecutionException ex) {
228233
throw new TaskRejectedException("Executor [" + this.scheduledExecutor + "] did not accept task: " + task, ex);
@@ -232,7 +237,8 @@ public ScheduledFuture<?> scheduleAtFixedRate(Runnable task, Instant startTime,
232237
@Override
233238
public ScheduledFuture<?> scheduleAtFixedRate(Runnable task, Duration period) {
234239
try {
235-
return this.scheduledExecutor.scheduleAtFixedRate(decorateTask(task, true), 0, period.toNanos(), TimeUnit.NANOSECONDS);
240+
return this.scheduledExecutor.scheduleAtFixedRate(decorateTask(task, true),
241+
0, NANO.convert(period), NANO);
236242
}
237243
catch (RejectedExecutionException ex) {
238244
throw new TaskRejectedException("Executor [" + this.scheduledExecutor + "] did not accept task: " + task, ex);
@@ -243,7 +249,8 @@ public ScheduledFuture<?> scheduleAtFixedRate(Runnable task, Duration period) {
243249
public ScheduledFuture<?> scheduleWithFixedDelay(Runnable task, Instant startTime, Duration delay) {
244250
Duration initialDelay = Duration.between(this.clock.instant(), startTime);
245251
try {
246-
return this.scheduledExecutor.scheduleWithFixedDelay(decorateTask(task, true), initialDelay.toNanos(), delay.toNanos(), TimeUnit.NANOSECONDS);
252+
return this.scheduledExecutor.scheduleWithFixedDelay(decorateTask(task, true),
253+
NANO.convert(initialDelay), NANO.convert(delay), NANO);
247254
}
248255
catch (RejectedExecutionException ex) {
249256
throw new TaskRejectedException("Executor [" + this.scheduledExecutor + "] did not accept task: " + task, ex);
@@ -253,7 +260,8 @@ public ScheduledFuture<?> scheduleWithFixedDelay(Runnable task, Instant startTim
253260
@Override
254261
public ScheduledFuture<?> scheduleWithFixedDelay(Runnable task, Duration delay) {
255262
try {
256-
return this.scheduledExecutor.scheduleWithFixedDelay(decorateTask(task, true), 0, delay.toNanos(), TimeUnit.NANOSECONDS);
263+
return this.scheduledExecutor.scheduleWithFixedDelay(decorateTask(task, true),
264+
0, NANO.convert(delay), NANO);
257265
}
258266
catch (RejectedExecutionException ex) {
259267
throw new TaskRejectedException("Executor [" + this.scheduledExecutor + "] did not accept task: " + task, ex);

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

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,9 @@
6363
public class ThreadPoolTaskScheduler extends ExecutorConfigurationSupport
6464
implements AsyncListenableTaskExecutor, SchedulingTaskExecutor, TaskScheduler {
6565

66+
private static final TimeUnit NANO = TimeUnit.NANOSECONDS;
67+
68+
6669
private volatile int poolSize = 1;
6770

6871
private volatile boolean removeOnCancelPolicy;
@@ -382,7 +385,8 @@ public ScheduledFuture<?> schedule(Runnable task, Instant startTime) {
382385
ScheduledExecutorService executor = getScheduledExecutor();
383386
Duration initialDelay = Duration.between(this.clock.instant(), startTime);
384387
try {
385-
return executor.schedule(errorHandlingTask(task, false), initialDelay.toNanos(), TimeUnit.NANOSECONDS);
388+
return executor.schedule(errorHandlingTask(task, false),
389+
NANO.convert(initialDelay), NANO);
386390
}
387391
catch (RejectedExecutionException ex) {
388392
throw new TaskRejectedException("Executor [" + executor + "] did not accept task: " + task, ex);
@@ -394,7 +398,8 @@ public ScheduledFuture<?> scheduleAtFixedRate(Runnable task, Instant startTime,
394398
ScheduledExecutorService executor = getScheduledExecutor();
395399
Duration initialDelay = Duration.between(this.clock.instant(), startTime);
396400
try {
397-
return executor.scheduleAtFixedRate(errorHandlingTask(task, true), initialDelay.toNanos(), period.toNanos(), TimeUnit.NANOSECONDS);
401+
return executor.scheduleAtFixedRate(errorHandlingTask(task, true),
402+
NANO.convert(initialDelay), NANO.convert(period), NANO);
398403
}
399404
catch (RejectedExecutionException ex) {
400405
throw new TaskRejectedException("Executor [" + executor + "] did not accept task: " + task, ex);
@@ -405,7 +410,8 @@ public ScheduledFuture<?> scheduleAtFixedRate(Runnable task, Instant startTime,
405410
public ScheduledFuture<?> scheduleAtFixedRate(Runnable task, Duration period) {
406411
ScheduledExecutorService executor = getScheduledExecutor();
407412
try {
408-
return executor.scheduleAtFixedRate(errorHandlingTask(task, true), 0, period.toNanos(), TimeUnit.NANOSECONDS);
413+
return executor.scheduleAtFixedRate(errorHandlingTask(task, true),
414+
0, NANO.convert(period), NANO);
409415
}
410416
catch (RejectedExecutionException ex) {
411417
throw new TaskRejectedException("Executor [" + executor + "] did not accept task: " + task, ex);
@@ -417,7 +423,8 @@ public ScheduledFuture<?> scheduleWithFixedDelay(Runnable task, Instant startTim
417423
ScheduledExecutorService executor = getScheduledExecutor();
418424
Duration initialDelay = Duration.between(this.clock.instant(), startTime);
419425
try {
420-
return executor.scheduleWithFixedDelay(errorHandlingTask(task, true), initialDelay.toNanos(), delay.toNanos(), TimeUnit.NANOSECONDS);
426+
return executor.scheduleWithFixedDelay(errorHandlingTask(task, true),
427+
NANO.convert(initialDelay), NANO.convert(delay), NANO);
421428
}
422429
catch (RejectedExecutionException ex) {
423430
throw new TaskRejectedException("Executor [" + executor + "] did not accept task: " + task, ex);
@@ -428,7 +435,8 @@ public ScheduledFuture<?> scheduleWithFixedDelay(Runnable task, Instant startTim
428435
public ScheduledFuture<?> scheduleWithFixedDelay(Runnable task, Duration delay) {
429436
ScheduledExecutorService executor = getScheduledExecutor();
430437
try {
431-
return executor.scheduleWithFixedDelay(errorHandlingTask(task, true), 0, delay.toNanos(), TimeUnit.NANOSECONDS);
438+
return executor.scheduleWithFixedDelay(errorHandlingTask(task, true),
439+
0, NANO.convert(delay), NANO);
432440
}
433441
catch (RejectedExecutionException ex) {
434442
throw new TaskRejectedException("Executor [" + executor + "] did not accept task: " + task, ex);

‎spring-context/src/test/java/org/springframework/scheduling/annotation/ScheduledAnnotationBeanPostProcessorTests.java

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2022 the original author or authors.
2+
* Copyright 2002-2023 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -95,6 +95,7 @@ void closeContextAfterTest() {
9595
FixedDelay, 5_000
9696
FixedDelayInSeconds, 5_000
9797
FixedDelayInMinutes, 180_000
98+
FixedDelayWithMaxValue, -1
9899
""")
99100
void fixedDelayTask(@NameToClass Class<?> beanClass, long expectedInterval) {
100101
BeanDefinition processorDefinition = new RootBeanDefinition(ScheduledAnnotationBeanPostProcessor.class);
@@ -120,7 +121,8 @@ void fixedDelayTask(@NameToClass Class<?> beanClass, long expectedInterval) {
120121
assertThat(targetObject).isEqualTo(target);
121122
assertThat(targetMethod.getName()).isEqualTo("fixedDelay");
122123
assertThat(task.getInitialDelayDuration()).isZero();
123-
assertThat(task.getIntervalDuration()).isEqualTo(Duration.ofMillis(expectedInterval));
124+
assertThat(task.getIntervalDuration()).isEqualTo(
125+
Duration.ofMillis(expectedInterval < 0 ? Long.MAX_VALUE : expectedInterval));
124126
}
125127

126128
@ParameterizedTest
@@ -343,8 +345,7 @@ void cronTaskWithInvalidZone() {
343345
BeanDefinition targetDefinition = new RootBeanDefinition(CronWithInvalidTimezoneTestBean.class);
344346
context.registerBeanDefinition("postProcessor", processorDefinition);
345347
context.registerBeanDefinition("target", targetDefinition);
346-
assertThatExceptionOfType(BeanCreationException.class).isThrownBy(
347-
context::refresh);
348+
assertThatExceptionOfType(BeanCreationException.class).isThrownBy(context::refresh);
348349
}
349350

350351
@Test
@@ -355,8 +356,7 @@ void cronTaskWithMethodValidation() {
355356
context.registerBeanDefinition("methodValidation", validationDefinition);
356357
context.registerBeanDefinition("postProcessor", processorDefinition);
357358
context.registerBeanDefinition("target", targetDefinition);
358-
assertThatExceptionOfType(BeanCreationException.class).isThrownBy(
359-
context::refresh);
359+
assertThatExceptionOfType(BeanCreationException.class).isThrownBy(context::refresh);
360360
}
361361

362362
@Test
@@ -702,18 +702,16 @@ void emptyAnnotation() {
702702
BeanDefinition targetDefinition = new RootBeanDefinition(EmptyAnnotationTestBean.class);
703703
context.registerBeanDefinition("postProcessor", processorDefinition);
704704
context.registerBeanDefinition("target", targetDefinition);
705-
assertThatExceptionOfType(BeanCreationException.class).isThrownBy(
706-
context::refresh);
705+
assertThatExceptionOfType(BeanCreationException.class).isThrownBy(context::refresh);
707706
}
708707

709708
@Test
710-
void invalidCron() throws Throwable {
709+
void invalidCron() {
711710
BeanDefinition processorDefinition = new RootBeanDefinition(ScheduledAnnotationBeanPostProcessor.class);
712711
BeanDefinition targetDefinition = new RootBeanDefinition(InvalidCronTestBean.class);
713712
context.registerBeanDefinition("postProcessor", processorDefinition);
714713
context.registerBeanDefinition("target", targetDefinition);
715-
assertThatExceptionOfType(BeanCreationException.class).isThrownBy(
716-
context::refresh);
714+
assertThatExceptionOfType(BeanCreationException.class).isThrownBy(context::refresh);
717715
}
718716

719717
@Test
@@ -722,8 +720,7 @@ void nonEmptyParamList() {
722720
BeanDefinition targetDefinition = new RootBeanDefinition(NonEmptyParamListTestBean.class);
723721
context.registerBeanDefinition("postProcessor", processorDefinition);
724722
context.registerBeanDefinition("target", targetDefinition);
725-
assertThatExceptionOfType(BeanCreationException.class).isThrownBy(
726-
context::refresh);
723+
assertThatExceptionOfType(BeanCreationException.class).isThrownBy(context::refresh);
727724
}
728725

729726

@@ -748,6 +745,13 @@ void fixedDelay() {
748745
}
749746
}
750747

748+
static class FixedDelayWithMaxValue {
749+
750+
@Scheduled(fixedDelay = Long.MAX_VALUE)
751+
void fixedDelay() {
752+
}
753+
}
754+
751755

752756
static class FixedRate {
753757

0 commit comments

Comments
 (0)