raboof/pekko
1
0
Fork 0
Code Issues Pull requests Projects Releases 6 Packages Wiki Activity Actions 10

fix: reject zero and negative periodic tasks schedule (#887)

Browse source 
* fix: reject zero and negative periodic tasks schedule

* fix: undo the symbol change

* use different test name, redescribe the exception

* abstract check function

* remove the printlns change

* reduce time units scale convert
This commit is contained in:
AndyChen 2024-01-18 19:47:13 +08:00 • committed by GitHub
parent 19da73673d
commit 7b2a3d8c27
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 74 additions and 9 deletions
Download patch file Download diff file Expand all files Collapse all files

57
actor-tests/src/test/scala/org/apache/pekko/actor/SchedulerSpec.scala
View file

@ -661,6 +661,45 @@ class LightArrayRevolverSchedulerSpec extends PekkoSpec(SchedulerSpec.testConfRe
}
}
"using scheduleAtFixedRate" must {
"fixed rate reject periodic tasks scheduled with zero interval" taggedAs TimingTest in {
val zeroInterval = 0.seconds
import system.dispatcher
intercept[IllegalArgumentException] {
system.scheduler.scheduleAtFixedRate(100.millis, zeroInterval, testActor, "Not Allowed")
}
expectNoMessage(1.second)
}
"fixed rate reject periodic tasks scheduled with negative interval" taggedAs TimingTest in {
val negativeDelay = -1.seconds
import system.dispatcher
intercept[IllegalArgumentException] {
system.scheduler.scheduleAtFixedRate(100.millis, negativeDelay, testActor, "Not Allowed")
}
expectNoMessage(1.second)
}
}
"using scheduleWithFixedDelay" must {
"fixed delay reject periodic tasks scheduled with zero interval" taggedAs TimingTest in {
val zeroInterval = 0.seconds
import system.dispatcher
intercept[IllegalArgumentException] {
system.scheduler.scheduleWithFixedDelay(100.millis, zeroInterval, testActor, "Not Allowed")
}
expectNoMessage(1.second)
}
"fixed delay reject periodic tasks scheduled with negative interval" taggedAs TimingTest in {
val negativeDelay = -1.seconds
import system.dispatcher
intercept[IllegalArgumentException] {
system.scheduler.scheduleWithFixedDelay(100.millis, negativeDelay, testActor, "Not Allowed")
}
expectNoMessage(1.second)
}
}
// same tests for fixedDelay and fixedRate
List(new ScheduleWithFixedDelayAdapter, new ScheduleAtFixedRateAdapter).foreach { scheduleAdapter =>
s"using $scheduleAdapter" must {
@ -684,6 +723,24 @@ class LightArrayRevolverSchedulerSpec extends PekkoSpec(SchedulerSpec.testConfRe
}
expectNoMessage(1.second)
}
"reject periodic tasks scheduled with zero interval" taggedAs TimingTest in {
val zeroInterval = 0.seconds
import system.dispatcher
intercept[IllegalArgumentException] {
scheduleAdapter.schedule(100.millis, zeroInterval, testActor, "Not Allowed")
}
expectNoMessage(1.second)
}
"reject periodic tasks scheduled with negative interval" taggedAs TimingTest in {
val negativeInterval = -1.seconds
import system.dispatcher
intercept[IllegalArgumentException] {
scheduleAdapter.schedule(100.millis, negativeInterval, testActor, "Not Allowed")
}
expectNoMessage(1.second)
}
}
}

8
actor/src/main/scala/org/apache/pekko/actor/LightArrayRevolverScheduler.scala
View file

@ -105,12 +105,14 @@ class LightArrayRevolverScheduler(config: Config, log: LoggingAdapter, threadFac
override def scheduleWithFixedDelay(initialDelay: FiniteDuration, delay: FiniteDuration)(runnable: Runnable)(
implicit executor: ExecutionContext): Cancellable = {
checkPeriod(delay)
checkMaxDelay(roundUp(delay).toNanos)
super.scheduleWithFixedDelay(initialDelay, delay)(runnable)
}
override def schedule(initialDelay: FiniteDuration, delay: FiniteDuration, runnable: Runnable)(
implicit executor: ExecutionContext): Cancellable = {
checkPeriod(delay)
checkMaxDelay(roundUp(delay).toNanos)
new AtomicCancellable(InitialRepeatMarker) { self =>
final override protected def scheduledFirst(): Cancellable =
@ -193,6 +195,12 @@ class LightArrayRevolverScheduler(config: Config, log: LoggingAdapter, threadFac
task
}
private def checkPeriod(delay: FiniteDuration): Unit =
if (delay.length <= 0)
throw new IllegalArgumentException(
s"Task scheduled with [${delay.toSeconds}] seconds delay, which means creating an infinite loop. " +
s"The expected delay must be greater than 0.")
private def checkMaxDelay(delayNanos: Long): Unit =
if (delayNanos / tickNanos > Int.MaxValue)
// 1 second margin in the error message due to rounding

18
actor/src/main/scala/org/apache/pekko/actor/Scheduler.scala
View file

@ -75,7 +75,7 @@ trait Scheduler {
* If the `Runnable` throws an exception the repeated scheduling is aborted,
* i.e. the function will not be invoked any more.
*
* @throws java.lang.IllegalArgumentException if the given delays exceed the maximum
* @throws java.lang.IllegalArgumentException if the given delays is zero, negative or exceed the maximum
* reach (calculated as: `delay / tickNanos > Int.MaxValue`).
*
* Note: For scheduling within actors `with Timers` should be preferred.
@ -116,7 +116,7 @@ trait Scheduler {
* If the `Runnable` throws an exception the repeated scheduling is aborted,
* i.e. the function will not be invoked any more.
*
* @throws java.lang.IllegalArgumentException if the given delays exceed the maximum
* @throws java.lang.IllegalArgumentException if the given delays is zero, negative or exceed the maximum
* reach (calculated as: `delay / tickNanos > Int.MaxValue`).
*
* Note: For scheduling within actors `AbstractActorWithTimers` should be preferred.
@ -215,7 +215,7 @@ trait Scheduler {
* If the `Runnable` throws an exception the repeated scheduling is aborted,
* i.e. the function will not be invoked any more.
*
* @throws java.lang.IllegalArgumentException if the given delays exceed the maximum
* @throws java.lang.IllegalArgumentException if the given delays is zero, negative or exceed the maximum
* reach (calculated as: `delay / tickNanos > Int.MaxValue`).
*
* Note: For scheduling within actors `with Timers` should be preferred.
@ -251,7 +251,7 @@ trait Scheduler {
* If the `Runnable` throws an exception the repeated scheduling is aborted,
* i.e. the function will not be invoked any more.
*
* @throws java.lang.IllegalArgumentException if the given delays exceed the maximum
* @throws java.lang.IllegalArgumentException if the given delays is zero, negative or exceed the maximum
* reach (calculated as: `delay / tickNanos > Int.MaxValue`).
*
* Note: For scheduling within actors `AbstractActorWithTimers` should be preferred.
@ -415,7 +415,7 @@ trait Scheduler {
* Scala API: Schedules a message to be sent once with a delay, i.e. a time period that has
* to pass before the message is sent.
*
* @throws java.lang.IllegalArgumentException if the given delays exceed the maximum
* @throws java.lang.IllegalArgumentException if the given delays is zero, negative or exceed the maximum
* reach (calculated as: `delay / tickNanos > Int.MaxValue`).
*
* Note: For scheduling within actors `with Timers` should be preferred.
@ -433,7 +433,7 @@ trait Scheduler {
* Java API: Schedules a message to be sent once with a delay, i.e. a time period that has
* to pass before the message is sent.
*
* @throws java.lang.IllegalArgumentException if the given delays exceed the maximum
* @throws java.lang.IllegalArgumentException if the given delays is zero, negative or exceed the maximum
* reach (calculated as: `delay / tickNanos > Int.MaxValue`).
*
* Note: For scheduling within actors `AbstractActorWithTimers` should be preferred.
@ -452,7 +452,7 @@ trait Scheduler {
* Scala API: Schedules a function to be run once with a delay, i.e. a time period that has
* to pass before the function is run.
*
* @throws java.lang.IllegalArgumentException if the given delays exceed the maximum
* @throws java.lang.IllegalArgumentException if the given delays is zero, negative or exceed the maximum
* reach (calculated as: `delay / tickNanos > Int.MaxValue`).
*
* Note: For scheduling within actors `with Timers` should be preferred.
@ -466,7 +466,7 @@ trait Scheduler {
* Scala API: Schedules a Runnable to be run once with a delay, i.e. a time period that
* has to pass before the runnable is executed.
*
* @throws java.lang.IllegalArgumentException if the given delays exceed the maximum
* @throws java.lang.IllegalArgumentException if the given delays is zero, negative or exceed the maximum
* reach (calculated as: `delay / tickNanos > Int.MaxValue`).
*
* Note: For scheduling within actors `with Timers` should be preferred.
@ -477,7 +477,7 @@ trait Scheduler {
* Java API: Schedules a Runnable to be run once with a delay, i.e. a time period that
* has to pass before the runnable is executed.
*
* @throws java.lang.IllegalArgumentException if the given delays exceed the maximum
* @throws java.lang.IllegalArgumentException if the given delays is zero, negative or exceed the maximum
* reach (calculated as: `delay / tickNanos > Int.MaxValue`).
*
* Note: For scheduling within actors `AbstractActorWithTimers` should be preferred.