Repository: go-co-op/gocron
Pull request #93 (2020-12-17 18:45:57)

add mutex to scheduler location
### What does this do?

protects the scheduler location struct field with a mutex to avoid possible data races of changing the scheduler location while the scheduler is running

### Which issue(s) does this PR fix/relate to?
<!--- Put `Resolves #XXX` here to auto-close the issue that your PR fixes (if such) --->


### List any changes that modify/break current functionality


### Have you included tests for your changes?


### Did you document any new/modified functionality?

- [x] Updated `example_test.go`
- [ ] Updated `README.md`

### Notes



Patch:
diff --git a/Makefile b/Makefile
index da2b262..08cbf82 100644
--- a/Makefile
+++ b/Makefile
@@ -30,4 +30,4 @@ vet:
 	@go vet $(GO_FLAGS) $(GO_PKGS)
 
 test:
-	@go test -race $(GO_FLAGS) -count=1 $(GO_PKGS)
+	@go test -race -v $(GO_FLAGS) -count=1 $(GO_PKGS)
diff --git a/scheduler.go b/scheduler.go
index e23541e..0721586 100644
--- a/scheduler.go
+++ b/scheduler.go
@@ -15,7 +15,8 @@ type Scheduler struct {
 	jobsMutex sync.RWMutex
 	jobs      []*Job
 
-	loc *time.Location
+	locationMutex sync.RWMutex
+	location      *time.Location
 
 	runningMutex sync.RWMutex
 	running      bool          // represents if the scheduler is running at the moment or not
@@ -28,7 +29,7 @@ type Scheduler struct {
 func NewScheduler(loc *time.Location) *Scheduler {
 	return &Scheduler{
 		jobs:     make([]*Job, 0),
-		loc:      loc,
+		location: loc,
 		running:  false,
 		stopChan: make(chan struct{}),
 		time:     &trueTime{},
@@ -111,12 +112,21 @@ func (s *Scheduler) Less(i, j int) bool {
 
 // ChangeLocation changes the default time location
 func (s *Scheduler) ChangeLocation(newLocation *time.Location) {
-	s.loc = newLocation
+	s.locationMutex.Lock()
+	defer s.locationMutex.Unlock()
+	s.location = newLocation
+}
+
+// Location provides the current location set on the scheduler
+func (s *Scheduler) Location() *time.Location {
+	s.locationMutex.RLock()
+	defer s.locationMutex.RUnlock()
+	return s.location
 }
 
 // scheduleNextRun Compute the instant when this Job should run next
 func (s *Scheduler) scheduleNextRun(job *Job) {
-	now := s.time.Now(s.loc)
+	now := s.time.Now(s.Location())
 
 	if job.neverRan() {
 		if !job.NextRun().IsZero() {
@@ -157,7 +167,7 @@ func (s *Scheduler) durationToNextRun(job *Job) time.Duration {
 
 func (s *Scheduler) getJobLastRun(job *Job) time.Time {
 	if job.neverRan() {
-		return s.time.Now(s.loc)
+		return s.time.Now(s.Location())
 	}
 	return job.LastRun()
 }
@@ -166,7 +176,7 @@ func (s *Scheduler) calculateMonths(job *Job, lastRun time.Time) time.Duration {
 	lastRunRoundedMidnight := s.roundToMidnight(lastRun)
 
 	if job.dayOfTheMonth > 0 { // calculate days to j.dayOfTheMonth
-		jobDay := time.Date(lastRun.Year(), lastRun.Month(), job.dayOfTheMonth, 0, 0, 0, 0, s.loc).Add(job.getAtTime())
+		jobDay := time.Date(lastRun.Year(), lastRun.Month(), job.dayOfTheMonth, 0, 0, 0, 0, s.Location()).Add(job.getAtTime())
 		daysDifference := int(math.Abs(lastRun.Sub(jobDay).Hours()) / 24)
 		nextRun := s.roundToMidnight(lastRun).Add(job.getAtTime())
 		if jobDay.Before(lastRun) { // shouldn't run this month; schedule for next interval minus day difference
@@ -203,7 +213,7 @@ func (s *Scheduler) calculateTotalDaysDifference(lastRun time.Time, daysToWeekda
 	}
 
 	if daysToWeekday == 0 { // today, at future time or already passed
-		lastRunAtTime := time.Date(lastRun.Year(), lastRun.Month(), lastRun.Day(), 0, 0, 0, 0, s.loc).Add(job.getAtTime())
+		lastRunAtTime := time.Date(lastRun.Year(), lastRun.Month(), lastRun.Day(), 0, 0, 0, 0, s.Location()).Add(job.getAtTime())
 		if lastRun.Before(lastRunAtTime) || lastRun.Equal(lastRunAtTime) {
 			return 0
 		}
@@ -215,13 +225,13 @@ func (s *Scheduler) calculateTotalDaysDifference(lastRun time.Time, daysToWeekda
 
 func (s *Scheduler) calculateDays(job *Job, lastRun time.Time) time.Duration {
 	if job.interval == 1 {
-		lastRunDayPlusJobAtTime := time.Date(lastRun.Year(), lastRun.Month(), lastRun.Day(), 0, 0, 0, 0, s.loc).Add(job.getAtTime())
+		lastRunDayPlusJobAtTime := time.Date(lastRun.Year(), lastRun.Month(), lastRun.Day(), 0, 0, 0, 0, s.Location()).Add(job.getAtTime())
 		if shouldRunToday(lastRun, lastRunDayPlusJobAtTime) {
 			return s.until(lastRun, s.roundToMidnight(lastRun).Add(job.getAtTime()))
 		}
 	}
 
-	nextRunAtTime := s.roundToMidnight(lastRun).Add(job.getAtTime()).AddDate(0, 0, int(job.interval)).In(s.loc)
+	nextRunAtTime := s.roundToMidnight(lastRun).Add(job.getAtTime()).AddDate(0, 0, int(job.interval)).In(s.Location())
 	return s.until(lastRun, nextRunAtTime)
 }
 
@@ -236,7 +246,7 @@ func shouldRunToday(lastRun time.Time, atTime time.Time) bool {
 func (s *Scheduler) calculateDuration(job *Job) time.Duration {
 	lastRun := job.LastRun()
 	if job.neverRan() && shouldRunAtSpecificTime(job) { // ugly. in order to avoid this we could prohibit setting .At() and allowing only .StartAt() when dealing with Duration types
-		atTime := time.Date(lastRun.Year(), lastRun.Month(), lastRun.Day(), 0, 0, 0, 0, s.loc).Add(job.getAtTime())
+		atTime := time.Date(lastRun.Year(), lastRun.Month(), lastRun.Day(), 0, 0, 0, 0, s.Location()).Add(job.getAtTime())
 		if lastRun.Before(atTime) || lastRun.Equal(atTime) {
 			return time.Until(s.roundToMidnight(lastRun).Add(job.getAtTime()))
 		}
@@ -267,7 +277,7 @@ func remainingDaysToWeekday(from time.Weekday, to time.Weekday) int {
 
 // roundToMidnight truncates time to midnight
 func (s *Scheduler) roundToMidnight(t time.Time) time.Time {
-	return time.Date(t.Year(), t.Month(), t.Day(), 0, 0, 0, 0, s.loc)
+	return time.Date(t.Year(), t.Month(), t.Day(), 0, 0, 0, 0, s.Location())
 }
 
 // Get the current runnable Jobs, which shouldRun is True
@@ -285,7 +295,7 @@ func (s *Scheduler) runnableJobs() []*Job {
 // NextRun datetime when the next Job should run.
 func (s *Scheduler) NextRun() (*Job, time.Time) {
 	if len(s.Jobs()) <= 0 {
-		return nil, s.time.Now(s.loc)
+		return nil, s.time.Now(s.Location())
 	}
 
 	sort.Sort(s)
@@ -316,7 +326,7 @@ func (s *Scheduler) runAndReschedule(job *Job) error {
 }
 
 func (s *Scheduler) run(job *Job) error {
-	job.setLastRun(s.time.Now(s.loc))
+	job.setLastRun(s.time.Now(s.Location()))
 	go job.run()
 	return nil
 }
@@ -483,7 +493,7 @@ func (s *Scheduler) shouldRun(j *Job) bool {
 		s.RemoveByReference(j)
 	}
 
-	return j.shouldRun() && s.time.Now(s.loc).Unix() >= j.NextRun().Unix()
+	return j.shouldRun() && s.time.Now(s.Location()).Unix() >= j.NextRun().Unix()
 }
 
 // setUnit sets the unit type


Test patch:
diff --git a/example_test.go b/example_test.go
index ec634ae..cbdb547 100644
--- a/example_test.go
+++ b/example_test.go
@@ -11,6 +11,27 @@ var task = func() {
 	fmt.Println("I am a task")
 }
 
+func ExampleScheduler_Location() {
+	s := gocron.NewScheduler(time.UTC)
+	fmt.Println(s.Location())
+	// Output: UTC
+}
+
+func ExampleScheduler_ChangeLocation() {
+	s := gocron.NewScheduler(time.UTC)
+	fmt.Println(s.Location())
+
+	location, err := time.LoadLocation("America/Los_Angeles")
+	if err != nil {
+		panic(err)
+	}
+	s.ChangeLocation(location)
+	fmt.Println(s.Location())
+	// Output:
+	// UTC
+	// America/Los_Angeles
+}
+
 func ExampleScheduler_StartBlocking() {
 	s := gocron.NewScheduler(time.UTC)
 	_, _ = s.Every(3).Seconds().Do(task)
@@ -43,13 +64,6 @@ func ExampleScheduler_At() {
 	_, _ = s.Every(1).Monday().At("10:30:01").Do(task)
 }
 
-func ExampleJob_ScheduledTime() {
-	s := gocron.NewScheduler(time.UTC)
-	job, _ := s.Every(1).Day().At("10:30").Do(task)
-	fmt.Println(job.ScheduledAtTime())
-	// Output: 10:30
-}
-
 func ExampleScheduler_RemoveJobByTag() {
 	s := gocron.NewScheduler(time.UTC)
 	tag1 := []string{"tag1"}
@@ -83,6 +97,13 @@ func ExampleScheduler_Clear() {
 	// 0
 }
 
+func ExampleJob_ScheduledTime() {
+	s := gocron.NewScheduler(time.UTC)
+	job, _ := s.Every(1).Day().At("10:30").Do(task)
+	fmt.Println(job.ScheduledAtTime())
+	// Output: 10:30
+}
+
 func ExampleJob_LimitRunsTo() {
 	s := gocron.NewScheduler(time.UTC)
 	job, _ := s.Every(1).Second().Do(task)
diff --git a/scheduler_test.go b/scheduler_test.go
index 2333fcc..6e04ae2 100644
--- a/scheduler_test.go
+++ b/scheduler_test.go
@@ -317,11 +317,11 @@ func TestLess(t *testing.T) {
 func TestSetLocation(t *testing.T) {
 	s := NewScheduler(time.FixedZone("UTC-8", -8*60*60))
 
-	assert.Equal(t, time.FixedZone("UTC-8", -8*60*60), s.loc)
+	assert.Equal(t, time.FixedZone("UTC-8", -8*60*60), s.Location())
 
 	s.ChangeLocation(time.UTC)
 
-	assert.Equal(t, time.UTC, s.loc)
+	assert.Equal(t, time.UTC, s.Location())
 }
 
 func TestClear(t *testing.T) {