Repository navigation
Skip months that are too short for a Quartz L-n offset in CronExpression - #37686
Open
waw666waw666 wants to merge 1 commit into
Open
waw666waw666 wants to merge 1 commit into
waw666waw666 wants to merge 1 commit into
Conversation
The "L-n" day-of-month adjuster subtracts n days from the last day of
the month. When the month has n days or fewer ("L-28" in February of a
non-leap year, "L-30" in a 30-day month), the result lands in the
previous month, before the temporal being adjusted. CronExpression then
returns that earlier date from next(), even when called with that date
itself or a later time, so a CronTrigger computes the same past
execution time over and over.
Move to the next month instead when the offset does not fit, as
dayOfWeekInMonth() already does for an nth day-of-week that overflows
the month.
Signed-off-by: waw666waw666 <[email protected]>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
CronExpressionreturns a time before the given one for a QuartzL-nday-of-month field when a month hasndays or fewer, and it keeps returning that same time, soCronTriggercomputes the same past execution over and over.With
new CronTrigger("0 0 12 L-28 * *"),nextExecution()keeps answering2025-01-31T12:00Zafter each run (I checked withSimpleTriggerContext, completion 50ms after the scheduled time), so on aThreadPoolTaskSchedulerthe task is rescheduled for a time in the past every time it completes, i.e. it runs back to back. This affectsL-28(February of non-leap years),L-29(February) andL-30(February and 30-day months).Cause
QuartzCronField.lastDayWithOffset(int)adds the negative offset to the last day of the month, so when the month is too short the result lands in the previous month. When the start is past the current month's ownL-nday,nextOrSame()rolls forward to the next month and adjusts again; if that month is too short, the second result falls back into the current month (January 31st forL-28in February 2025) and is returned without another check.CronExpressiontakes it as the next match, and the later fields only move it within that day, which is hownext()ends up earlier than its input.Fix
When the result is not in the month being adjusted, try the first day of the next month instead, up to 12 times, the same way
dayOfWeekInMonth()already handles an nth day-of-week that overflows the month. Months without such a day are skipped (as in Quartz); an offset that never fits (e.g.L-31) gives no next execution.Tests
CronExpressionTests#quartzLastDayOfMonthOffsetLongerThanMonth(L-28across February 2025,L-30across February and April 2024, and a start time after the bogus result). It fails onmain(expected: 2025-03-03T12:00 but was: 2025-01-31T12:00) and passes with the change.org.springframework.scheduling.supportpass, and Checkstyle is clean forspring-context.next()against a brute-force day-by-day search forL-1toL-35from start times spread over 2023 to 2025: about 20k mismatches before, none after.