Skip to content

Commit dcab190

Browse files
committed
Guard executor background goroutine channel writes with executorDone
Retry timer goroutines and task goroutines wrote to internalRetryCh and completionCh unconditionally. If Execute exits while these goroutines are still active, writes to abandoned channels relied on buffer sizing as an implicit safety net rather than explicit shutdown signalling. Add an executorDone channel closed via defer when Execute returns. Both the retry timer goroutine (internalRetryCh write) and task goroutines (completionCh write) now select on executorDone so they exit cleanly without writing to a channel no longer being drained.
1 parent f9e8ba1 commit dcab190

1 file changed

Lines changed: 20 additions & 6 deletions

File tree

executor/executor.go

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -102,13 +102,18 @@ func (e *Executor) Execute(ctx context.Context) error {
102102
// Using a channel ensures status mutations only happen on the main goroutine, avoiding data races.
103103
internalRetryCh := make(chan *taskRun, len(e.tasks))
104104

105+
// executorDone is closed when Execute returns, allowing background goroutines to detect
106+
// shutdown and avoid writing to channels that are no longer being read.
107+
executorDone := make(chan struct{})
108+
defer close(executorDone)
109+
105110
// Semaphore for max parallelization
106111
var semaphore chan struct{}
107112
if e.opts.MaxParallelization > 0 {
108113
semaphore = make(chan struct{}, e.opts.MaxParallelization)
109114
}
110115

111-
if err := e.startReadyTasks(ctx, completionCh, outputCh, semaphore); err != nil {
116+
if err := e.startReadyTasks(ctx, completionCh, outputCh, semaphore, executorDone); err != nil {
112117
return err
113118
}
114119

@@ -158,7 +163,10 @@ func (e *Executor) Execute(ctx context.Context) error {
158163
delay := parseRetryDelay(run.task.Retry.Delay, run.task.Retry.Backoff, run.attempt)
159164
go func(r *taskRun, d time.Duration) {
160165
time.Sleep(d)
161-
internalRetryCh <- r
166+
select {
167+
case internalRetryCh <- r:
168+
case <-executorDone:
169+
}
162170
}(run, delay)
163171
} else {
164172
run.status = statusFailed
@@ -181,7 +189,7 @@ func (e *Executor) Execute(ctx context.Context) error {
181189
}
182190

183191
if finished < total {
184-
if err := e.startReadyTasks(ctx, completionCh, outputCh, semaphore); err != nil {
192+
if err := e.startReadyTasks(ctx, completionCh, outputCh, semaphore, executorDone); err != nil {
185193
return err
186194
}
187195
if err := e.ensureProgress(); err != nil {
@@ -196,7 +204,7 @@ func (e *Executor) Execute(ctx context.Context) error {
196204
}
197205

198206
// startReadyTasks finds all pending tasks whose deps are satisfied and starts them
199-
func (e *Executor) startReadyTasks(ctx context.Context, completionCh chan<- CommandResult, outputCh chan<- Output, sem chan struct{}) error {
207+
func (e *Executor) startReadyTasks(ctx context.Context, completionCh chan<- CommandResult, outputCh chan<- Output, sem chan struct{}, executorDone <-chan struct{}) error {
200208
for name, run := range e.tasks {
201209
if run.status != statusPending {
202210
continue
@@ -243,19 +251,25 @@ func (e *Executor) startReadyTasks(ctx context.Context, completionCh chan<- Comm
243251
if e.opts.OnTaskFinish != nil {
244252
e.opts.OnTaskFinish(r.task.Name, err, elapsed)
245253
}
246-
completionCh <- CommandResult{
254+
select {
255+
case completionCh <- CommandResult{
247256
Name: r.task.Name,
248257
Success: false,
249258
Error: fmt.Errorf("task %s failed: %w", r.task.Name, err),
259+
}:
260+
case <-executorDone:
250261
}
251262
return
252263
}
253264
if e.opts.OnTaskFinish != nil {
254265
e.opts.OnTaskFinish(r.task.Name, nil, elapsed)
255266
}
256-
completionCh <- CommandResult{
267+
select {
268+
case completionCh <- CommandResult{
257269
Name: r.task.Name,
258270
Success: true,
271+
}:
272+
case <-executorDone:
259273
}
260274
}(run)
261275
}

0 commit comments

Comments
 (0)