Skip to content

Commit 0bbde1e

Browse files
committed
Remove data race in IsRunning method
This commit locks the job struct when reading or writting to the isRunning boolean. Also this adds -race to the travis check so this doesn't happen again in the future. Fixes #4
1 parent d39e4af commit 0bbde1e

File tree

3 files changed

+29
-26
lines changed

3 files changed

+29
-26
lines changed

.travis.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,4 +11,4 @@ install:
1111
- if ! go get code.google.com/p/go.tools/cmd/cover; then go get golang.org/x/tools/cmd/cover; fi
1212
- go get github.com/stretchr/testify/assert
1313
script:
14-
- $HOME/gopath/bin/goveralls -service=travis-ci -repotoken $COVERALLS_TOKEN
14+
- $HOME/gopath/bin/goveralls -race -service=travis-ci -repotoken $COVERALLS_TOKEN

scheduler.go

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import (
1818
"errors"
1919
"strconv"
2020
"strings"
21+
"sync"
2122
"time"
2223
)
2324

@@ -33,6 +34,7 @@ type Job struct {
3334
err error
3435
schedule scheduled
3536
isRunning bool
37+
sync.RWMutex
3638
}
3739

3840
type recurrent struct {
@@ -175,24 +177,32 @@ func (j *Job) Run(f func()) (*Job, error) {
175177
case <-j.Quit:
176178
return
177179
case <-j.SkipWait:
178-
go func(j *Job) {
179-
j.isRunning = true
180-
j.fn()
181-
j.isRunning = false
182-
}(j)
180+
go runJob(j)
183181
case <-time.After(next):
184-
go func(j *Job) {
185-
j.isRunning = true
186-
j.fn()
187-
j.isRunning = false
188-
}(j)
182+
go runJob(j)
189183
}
190184
next, _ = j.schedule.nextRun()
191185
}
192186
}(j)
193187
return j, nil
194188
}
195189

190+
func (j *Job) setRunning(running bool) {
191+
j.Lock()
192+
defer j.Unlock()
193+
194+
j.isRunning = running
195+
}
196+
197+
func runJob(job *Job) {
198+
if job.IsRunning() {
199+
return
200+
}
201+
job.setRunning(true)
202+
job.fn()
203+
job.setRunning(false)
204+
}
205+
196206
func parseTime(str string) (hour, min, sec int, err error) {
197207
chunks := strings.Split(str, ":")
198208
var hourStr, minStr, secStr string
@@ -311,5 +321,7 @@ func (j *Job) Hours() *Job {
311321

312322
// IsRunning returns if the job is currently running
313323
func (j *Job) IsRunning() bool {
324+
j.RLock()
325+
defer j.RUnlock()
314326
return j.isRunning
315327
}

scheduler_test.go

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -20,22 +20,13 @@ func TestIsRunning(t *testing.T) {
2020
assert.Nil(t, err)
2121
assert.NotNil(t, job)
2222

23-
var i int
24-
25-
FOR:
26-
for {
27-
time.Sleep(20 * time.Millisecond)
28-
i++
29-
30-
switch i {
31-
case 1:
32-
assert.Equal(t, true, job.IsRunning())
23+
time.Sleep(20 * time.Millisecond)
24+
assert.Equal(t, true, job.IsRunning())
25+
job.SkipWait <- true
26+
assert.Equal(t, true, job.IsRunning())
3327

34-
case 2:
35-
assert.Equal(t, false, job.IsRunning())
36-
break FOR
37-
}
38-
}
28+
time.Sleep(20 * time.Millisecond)
29+
assert.Equal(t, false, job.IsRunning())
3930
}
4031

4132
func TestExecution(t *testing.T) {

0 commit comments

Comments
 (0)