Skip to content

Commit ad027f8

Browse files
committed
fix: go test -race findings
1 parent 039d4f7 commit ad027f8

File tree

2 files changed

+45
-16
lines changed

2 files changed

+45
-16
lines changed

core/runjob.go

+22-12
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package core
33
import (
44
"fmt"
55
"strconv"
6+
"sync"
67
"time"
78

89
docker "github.com/fsouza/go-dockerclient"
@@ -38,12 +39,25 @@ type RunJob struct {
3839
Environment []string
3940

4041
containerID string
42+
mu sync.RWMutex // Protect containerID access
4143
}
4244

4345
func NewRunJob(c *docker.Client) *RunJob {
4446
return &RunJob{Client: c}
4547
}
4648

49+
func (j *RunJob) setContainerID(id string) {
50+
j.mu.Lock()
51+
j.containerID = id
52+
j.mu.Unlock()
53+
}
54+
55+
func (j *RunJob) getContainerID() string {
56+
j.mu.RLock()
57+
defer j.mu.RUnlock()
58+
return j.containerID
59+
}
60+
4761
func (j *RunJob) Run(ctx *Context) error {
4862
var container *docker.Container
4963
var err error
@@ -103,7 +117,7 @@ func (j *RunJob) Run(ctx *Context) error {
103117
}
104118

105119
if container != nil {
106-
j.containerID = container.ID
120+
j.setContainerID(container.ID)
107121
}
108122

109123
// cleanup container if it is a created one
@@ -126,7 +140,7 @@ func (j *RunJob) Run(ctx *Context) error {
126140
}
127141

128142
if logsErr := j.Client.Logs(docker.LogsOptions{
129-
Container: container.ID,
143+
Container: j.getContainerID(),
130144
OutputStream: ctx.Execution.OutputStream,
131145
ErrorStream: ctx.Execution.ErrorStream,
132146
Stdout: true,
@@ -205,19 +219,16 @@ func (j *RunJob) buildContainer() (*docker.Container, error) {
205219
}
206220

207221
func (j *RunJob) startContainer() error {
208-
return j.Client.StartContainer(j.containerID, &docker.HostConfig{})
222+
return j.Client.StartContainer(j.getContainerID(), &docker.HostConfig{})
209223
}
210224

211225
func (j *RunJob) stopContainer(timeout uint) error {
212-
return j.Client.StopContainer(j.containerID, timeout)
226+
return j.Client.StopContainer(j.getContainerID(), timeout)
213227
}
214228

215229
func (j *RunJob) getContainer() (*docker.Container, error) {
216-
container, err := j.Client.InspectContainer(j.containerID)
217-
if err != nil {
218-
return nil, err
219-
}
220-
return container, nil
230+
id := j.getContainerID()
231+
return j.Client.InspectContainer(id)
221232
}
222233

223234
const (
@@ -236,7 +247,7 @@ func (j *RunJob) watchContainer() error {
236247
return ErrMaxTimeRunning
237248
}
238249

239-
c, err := j.Client.InspectContainer(j.containerID)
250+
c, err := j.Client.InspectContainer(j.getContainerID())
240251
if err != nil {
241252
return err
242253
}
@@ -261,8 +272,7 @@ func (j *RunJob) deleteContainer() error {
261272
if delete, _ := strconv.ParseBool(j.Delete); !delete {
262273
return nil
263274
}
264-
265275
return j.Client.RemoveContainer(docker.RemoveContainerOptions{
266-
ID: j.containerID,
276+
ID: j.getContainerID(),
267277
})
268278
}

core/scheduler.go

+23-4
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ type Scheduler struct {
2121
cron *cron.Cron
2222
wg sync.WaitGroup
2323
isRunning bool
24+
mu sync.RWMutex // Protect isRunning and wg operations
2425
}
2526

2627
func NewScheduler(l Logger) *Scheduler {
@@ -60,21 +61,28 @@ func (s *Scheduler) RemoveJob(j Job) error {
6061
}
6162

6263
func (s *Scheduler) Start() error {
63-
s.Logger.Debugf("Starting scheduler")
64+
s.mu.Lock()
6465
s.isRunning = true
66+
s.mu.Unlock()
67+
s.Logger.Debugf("Starting scheduler")
6568
s.cron.Start()
6669
return nil
6770
}
6871

6972
func (s *Scheduler) Stop() error {
70-
s.wg.Wait()
71-
s.cron.Stop()
73+
s.cron.Stop() // Stop cron first to prevent new jobs
74+
75+
s.mu.Lock()
7276
s.isRunning = false
77+
s.mu.Unlock()
7378

79+
s.wg.Wait() // Then wait for existing jobs
7480
return nil
7581
}
7682

7783
func (s *Scheduler) IsRunning() bool {
84+
s.mu.RLock()
85+
defer s.mu.RUnlock()
7886
return s.isRunning
7987
}
8088

@@ -84,8 +92,19 @@ type jobWrapper struct {
8492
}
8593

8694
func (w *jobWrapper) Run() {
95+
w.s.mu.Lock()
96+
if !w.s.isRunning {
97+
w.s.mu.Unlock()
98+
return
99+
}
87100
w.s.wg.Add(1)
88-
defer w.s.wg.Done()
101+
w.s.mu.Unlock()
102+
103+
defer func() {
104+
w.s.mu.Lock()
105+
w.s.wg.Done()
106+
w.s.mu.Unlock()
107+
}()
89108

90109
e := NewExecution()
91110
ctx := NewContext(w.s, w.j, e)

0 commit comments

Comments
 (0)