Skip to content

Commit 2540cb2

Browse files
authored
Correctly abort on interrupt instead of going into endless loop (#439)
Running `:GoMetaLinter` just pointed me to this "ineffective break statement" that I introduced in #328 with the following comment: > It also makes sure that we don't enqueue more tasks/steps after being > canceled: that's the code in `(*executor).Start()` that aborts the > `Range` call if the context has been cancelled/timed-out and aborts > execution of a task if the goroutine already blocked on > `x.par.Acquire()`. Well... turns out that wasn't the case, was it? Instead the code only broke out of the `select` and continued trying to acquire a lock. I also suspect that this bug might have something to do with a customer reporting that long-running containers are not stopped when hitting Ctrl-C (https://github.com/sourcegraph/sourcegraph/issues/16026) even after we thought the issue was fixed with #328. I'm not 100% sure on this one though.
1 parent 3106715 commit 2540cb2

File tree

1 file changed

+3
-3
lines changed

1 file changed

+3
-3
lines changed

internal/campaigns/executor.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -192,10 +192,12 @@ func (x *executor) LogFiles() []string {
192192
}
193193

194194
func (x *executor) Start(ctx context.Context) {
195+
defer func() { close(x.doneEnqueuing) }()
196+
195197
for _, task := range x.tasks {
196198
select {
197199
case <-ctx.Done():
198-
break
200+
return
199201
default:
200202
}
201203

@@ -215,8 +217,6 @@ func (x *executor) Start(ctx context.Context) {
215217
}
216218
}(task)
217219
}
218-
219-
close(x.doneEnqueuing)
220220
}
221221

222222
func (x *executor) Wait() ([]*ChangesetSpec, error) {

0 commit comments

Comments
 (0)