From 5bc0245da6eea978eba585bad33e8ac228198377 Mon Sep 17 00:00:00 2001 From: John Roesler Date: Tue, 30 May 2023 12:00:42 -0500 Subject: [PATCH] fix: atTime being incorrectly set for non compatible units (#502) * fix: atTime being incorrectly set for non compatible units * fix race --- job.go | 4 ++++ scheduler.go | 18 ++++++++++-------- scheduler_test.go | 14 ++++++++++++++ 3 files changed, 28 insertions(+), 8 deletions(-) diff --git a/job.go b/job.go index fdada82e..30fb69e1 100644 --- a/job.go +++ b/job.go @@ -252,10 +252,14 @@ func (j *Job) addAtTime(t time.Duration) { } func (j *Job) getStartAtTime() time.Time { + j.mu.RLock() + defer j.mu.RUnlock() return j.startAtTime } func (j *Job) setStartAtTime(t time.Time) { + j.mu.Lock() + defer j.mu.Unlock() j.startAtTime = t } diff --git a/scheduler.go b/scheduler.go index 72850f41..25feb61b 100644 --- a/scheduler.go +++ b/scheduler.go @@ -205,7 +205,7 @@ func (s *Scheduler) scheduleNextRun(job *Job) (bool, nextRun) { // Increment startAtTime to the future if !job.startAtTime.IsZero() && job.startAtTime.Before(now) { duration := s.durationToNextRun(job.startAtTime, job).duration - job.startAtTime = job.startAtTime.Add(duration) + job.setStartAtTime(job.startAtTime.Add(duration)) if job.startAtTime.Before(now) { diff := now.Sub(job.startAtTime) duration := s.durationToNextRun(job.startAtTime, job).duration @@ -213,7 +213,7 @@ func (s *Scheduler) scheduleNextRun(job *Job) (bool, nextRun) { if diff%duration != 0 { count++ } - job.startAtTime = job.startAtTime.Add(duration * count) + job.setStartAtTime(job.startAtTime.Add(duration * count)) } } } else { @@ -248,12 +248,14 @@ func (s *Scheduler) durationToNextRun(lastRun time.Time, job *Job) nextRun { // job can be scheduled with .StartAt() if job.getFirstAtTime() == 0 && job.getStartAtTime().After(lastRun) { sa := job.getStartAtTime() - job.addAtTime( - time.Duration(sa.Hour())*time.Hour + - time.Duration(sa.Minute())*time.Minute + - time.Duration(sa.Second())*time.Second, - ) - return nextRun{duration: job.getStartAtTime().Sub(s.now()), dateTime: job.getStartAtTime()} + if job.unit == days || job.unit == weeks || job.unit == months { + job.addAtTime( + time.Duration(sa.Hour())*time.Hour + + time.Duration(sa.Minute())*time.Minute + + time.Duration(sa.Second())*time.Second, + ) + } + return nextRun{duration: sa.Sub(s.now()), dateTime: sa} } var next nextRun diff --git a/scheduler_test.go b/scheduler_test.go index 7c262bb1..00855d7a 100644 --- a/scheduler_test.go +++ b/scheduler_test.go @@ -1894,6 +1894,20 @@ func TestScheduler_Update(t *testing.T) { assert.Equal(t, expectedNext, actualNext) }) + + // Verifies https://github.com/go-co-op/gocron/issues/499 + t.Run("at time is not set when updating incompatible unit jobs", func(t *testing.T) { + s := NewScheduler(time.UTC) + startAt := time.Now().UTC().Add(time.Millisecond * 250) + j, err := s.Every(24 * time.Hour).StartAt(startAt).Do(func() {}) + require.NoError(t, err) + s.StartAsync() + time.Sleep(time.Millisecond * 500) + + _, err = s.Job(j).StartAt(startAt.Add(1 * time.Second)).Update() + require.NoError(t, err) + s.Stop() + }) } func TestScheduler_RunByTag(t *testing.T) {