Give opts.Stop priority in NewCmdTask's read loop

TestNewCmdTaskInstantStop is flaky: it closes the stop channel from
within start() and asserts the stopped task touched nothing. But Go's
select picks uniformly at random among ready cases, so when opts.Stop
and a data channel are both ready the loop can pick the data channel,
call beforeStart() (which clears the view) and write the prefix before
bailing. In production a task that's already been superseded thereby
clobbers the output the incoming task is about to render.

Check stop with a non-blocking select before each blocking select, so
the stop signal wins whenever it's already closed (Go has no built-in
priority select; this is the idiomatic substitute). The selects keep
their own stop case for liveness, to unblock when stop closes while
parked waiting for data.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Stefan Haller 2026-06-30 17:13:01 +02:00
parent e82dbf0fc6
commit adaef97ffe

View file

@ -248,8 +248,26 @@ func (self *ViewBufferManager) NewCmdTask(start func() (*exec.Cmd, io.Reader), p
}
}
// Go's select picks randomly among ready cases, so once opts.Stop is
// closed the selects below could still service a ready data channel
// instead of bailing. Check stop explicitly first to give it priority:
// a task that's been stopped (it's being replaced by a newer one) must
// not touch the view here — beforeStart clears it and the prefix gets
// written, clobbering what the incoming task is about to render.
stopped := func() bool {
select {
case <-opts.Stop:
return true
default:
return false
}
}
outer:
for {
if stopped() {
break outer
}
select {
case <-opts.Stop:
break outer
@ -260,6 +278,10 @@ func (self *ViewBufferManager) NewCmdTask(start func() (*exec.Cmd, io.Reader), p
}
}
for i := 0; linesToRead.Total == -1 || i < linesToRead.Total; i++ {
if stopped() {
callThen()
break outer
}
var ok bool
var line []byte
select {