diff --git a/async/taskhelper/cron_runner.go b/async/taskhelper/cron_runner.go index bd7eaeb..e120300 100644 --- a/async/taskhelper/cron_runner.go +++ b/async/taskhelper/cron_runner.go @@ -78,6 +78,16 @@ func (r *cronRunner) cron(ctx context.Context, cancelFunc context.CancelFunc) { now := time.Now() next := r.schedule.Next(now) for { + // Next returns the zero time when no date matches within five years. + // This can happen after a firing even though NewCron rejected schedules + // that never match at creation time (e.g. a leap-day schedule whose next + // match is more than five years away because 2100 is not a leap year). + // Arming a timer with a negative duration would execute the task in a + // busy loop, so stop the runner instead. + if next.IsZero() { + logrus.Errorf("task %s has no next execution date for its cron schedule, stopping the runner", simpleTask.String()) + return + } // After an execution, next is computed from the moment it started, so the timer is armed with the time left until this date. // That way, the time spent executing the task doesn't delay the next execution. // If the execution took longer than that, the next date has already passed, and the task is executed again right away. diff --git a/async/taskhelper/periodic_test.go b/async/taskhelper/periodic_test.go index dd0ca69..410847f 100644 --- a/async/taskhelper/periodic_test.go +++ b/async/taskhelper/periodic_test.go @@ -22,6 +22,7 @@ import ( "testing" "time" + "github.com/flc1125/go-cron/v4" "github.com/perses/common/async" "github.com/sirupsen/logrus" logrusTest "github.com/sirupsen/logrus/hooks/test" @@ -171,6 +172,45 @@ func TestNewCron_Schedule(t *testing.T) { } } +func TestCron_StopsWhenNoNextExecutionDate(t *testing.T) { + origHooks := logrus.StandardLogger().ReplaceHooks(make(logrus.LevelHooks)) + t.Cleanup(func() { + logrus.StandardLogger().ReplaceHooks(origHooks) + }) + hook := logrusTest.NewLocal(logrus.StandardLogger()) + + // A schedule that never matches any date: NewCron rejects it, so the runner + // is built directly to simulate the case where Next turns to zero after a + // firing (e.g. a leap-day schedule past its last match within five years). + sch, err := cron.ParseStandard("0 0 31 2 *") + require.NoError(t, err) + task := &failingTask{} + helper := &cronRunner{ + schedule: sch, + task: task, + isSimpleTask: true, + done: make(chan struct{}), + } + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + Run(ctx, cancel, helper) + select { + case <-helper.Done(): + case <-time.After(5 * time.Second): + t.Fatal("the runner has not stopped") + } + // The task is never executed: without the guard it would run in a busy loop. + assert.Equal(t, int32(0), task.calls.Load()) + // The stop is reported as an error. + found := false + for _, entry := range hook.AllEntries() { + if entry.Level == logrus.ErrorLevel { + found = true + } + } + assert.True(t, found, "an error should be logged when the runner stops") +} + func TestNew_ReturnTheExecuteError(t *testing.T) { task := &failingTask{failures: 1} helper, err := New(task)