Skip to content

Commit

Permalink
Avoid croniter infinite loop (#5957)
Browse files Browse the repository at this point in the history
* Avoid croniter loop

* Update test_clocks.py

* Create PR5957.yaml

* mypy

* Update clocks.py
  • Loading branch information
jlowin committed Jul 3, 2022
1 parent 3139839 commit 701e7c5
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 1 deletion.
21 changes: 21 additions & 0 deletions changes/PR5957.yaml
@@ -0,0 +1,21 @@
# An example changelog entry
#
# 1. Choose one (or more if a PR encompasses multiple changes) of the following headers:
# - feature
# - enhancement
# - task
# - fix
# - deprecation
# - breaking (for breaking changes)
#
# 2. Fill in one (or more) bullet points under the heading, describing the change.
# Markdown syntax may be used.
#
# 3. If you would like to be credited as helping with this release, add a
# contributor section with your name and github username.
#
# Here's an example of a PR that adds an enhancement

fix:
- "Fix bug with infinite loop when parsing DST cron schedules - [#5957](https://github.com/PrefectHQ/prefect/pull/5957)"

20 changes: 19 additions & 1 deletion src/prefect/schedules/clocks.py
Expand Up @@ -317,13 +317,31 @@ def events(self, after: datetime = None) -> Iterable[ClockEvent]:
cron = croniter(self.cron, after_localized, day_or=self.day_or) # type: ignore
dates = set() # type: Set[datetime]

iterations = 0
while True:
iterations += 1
# escape hatch
if iterations > 1000:
break
next_date = pendulum.instance(cron.get_next(datetime))
# because of croniter's rounding behavior, we want to avoid
# issuing the after date; we also want to avoid duplicates caused by
# DST boundary issues
if next_date.in_tz("UTC") == after.in_tz("UTC") or next_date in dates:
stuck_count = 0
while next_date.in_tz("UTC") == after.in_tz("UTC") or next_date in dates:
next_date = pendulum.instance(cron.get_next(datetime))
stuck_count += 1

# if we are stuck in a DST loop, create a new cron instance
# that starts after this date. This brute force avoids
# croniter bugs like the Santiago DST boundary
if stuck_count == 3:
after = next_date
cron = croniter(self.cron, after, day_or=self.day_or) # type: ignore
next_date = pendulum.instance(cron.get_next(datetime))
# if we're still seeing the issue, escape!
if next_date in dates:
iterations = 1000

if self.end_date and next_date > self.end_date:
break
Expand Down
20 changes: 20 additions & 0 deletions tests/schedules/test_clocks.py
Expand Up @@ -640,6 +640,26 @@ def test_cron_clock_daily_start_daylight_savings_time_backward(self, serialize):
]
assert [t.start_time.in_tz("UTC").hour for t in next_4] == [13, 13, 14, 14]

@pytest.mark.parametrize("serialize", [True, False])
def test_cron_clock_santiago(self, serialize):
"""
On 9/4/2022, at 12am, America/Santiago switches clocks back an hour.
Croniter gets stuck in an infinite loop at midnight
"""
dt = pendulum.datetime(2022, 8, 21, 10, tz="America/Santiago")
c = clocks.CronClock("0 0 * * 0", dt)
if serialize:
c = ClockSchema().load(ClockSchema().dump(c))
next_4 = islice(c.events(after=dt), 4)
# no infinite loop on the 4th
assert [t.start_time.in_tz("America/Santiago").day for t in next_4] == [
28,
4,
11,
18,
]


class TestDatesClock:
def test_create_dates_clock_one_date(self):
Expand Down

0 comments on commit 701e7c5

Please sign in to comment.