watch: timeout path leaks one goroutine per kill and races on the shared output buffer

command/watch.go の doWatch と evalCmdOutput から始め、次に提供されている gotify watch の再現手順を race detector で実行します。繰り返される timeout の tick によって goroutine が蓄積せず、child とその出力をコピーする処理が完了した後にのみ出力が読み取られるようになれば、issue は完了です。

Summary

In watch mode, when the watched command exceeds the interval and is killed via the timeout path, evalCmdOutput leaks a goroutine per timeout occurrence and reads the shared output buffer concurrently with the still-running child pipeline, which is a data race.

Location

• File: command/watch.go
• Function: doWatch → evalCmdOutput (lines ~86–108)

done := make(chan error)          // unbuffered
go func() {
    err := cmd.Wait()
    if err != nil {
        done <- fmt.Errorf("command failed to invoke: %v", err)
    }
    done <- nil
}()
select {
case err := <-done:
    return outputBuf.String(), err
case <-timeOut:
    cmd.Process.Kill()
    return outputBuf.String(), errors.New("command timed out")
}

Problem

Two defects on the timeout path:

1. Goroutine leak. done is unbuffered, and the select abandons it when timeOut fires. The spawned goroutine calls cmd.Wait(); once the killed process is reaped, Wait returns and the goroutine attempts done <- ... with no receiver ever coming, so it blocks forever. Every interval tick that times out leaks one goroutine (plus the reaped-process bookkeeping), for as long as gotify watch runs — which is by design unbounded (for range time.NewTicker(...).C).

2. Data race on outputBuf. When Stdout/Stderr are non-*os.File writers (here a *bytes.Buffer), os/exec spawns internal copy goroutines that write into outputBuf, and normally only cmd.Wait() provides the happens-before edge that makes reading the buffer safe. On the timeout path Process.Kill() is called without Wait(), and outputBuf.String() runs concurrently with those copy goroutines, which may still be draining buffered pipe data from the dying child. Concurrent Buffer.Write and Buffer.String are not safe.

Trigger / Reproduction

Static analysis finding — not confirmed by execution. Run:

gotify watch -n 1 -- sh -c 'sleep 5; echo done'

Every tick kills sleep 5 after 1 s, taking the case <-timeOut: branch each time: one blocked-forever goroutine per tick, plus an unsynchronized read of outputBuf.

Expected Behavior

The timeout path should reap the child (e.g. call cmd.Wait() after Kill() in a separate goroutine, or use exec.CommandContext with a per-run context so cancellation/reaping is handled by the stdlib) and only then read outputBuf; the completion goroutine should never block on an unreceived channel send (e.g. done := make(chan error, 1)).

Actual Behavior

Goroutines accumulate indefinitely across ticks, and outputBuf can be read while exec's copy goroutines are still writing to it.

Impact

Long-running gotify watch deployments that monitor slow commands accumulate leaked goroutines over time (unbounded memory growth). The unsynchronized buffer access is undefined behavior under the Go memory model and can, under -race, flag crashes/misreads; practically it can observe torn/partial output.

Suggested Direction

Minimal fix: make done buffered (make(chan error, 1)), and after cmd.Process.Kill() call <-done (or cmd.Wait()) before returning outputBuf.String() so the child is fully reaped before its output is read. A cleaner alternative is switching to exec.CommandContext(ctx, ...) per run.

Related but distinct: issue #44 asks for a way to disable the timeout entirely; this report concerns the correctness of the existing timeout implementation itself.