Build concurrency is unbounded: buildRun cannot cancel an in-flight build (same as #473)

Summary

buildRun cannot cancel a build that is already running, so build concurrency is unbounded: one additional full go build per debounce window, for as long as the developer keeps saving. On a large project this is enough to exhaust a machine. In our case it reached 45 concurrent go build invocations, 84 compile children, ~24 GB resident on a 30 GB laptop, and systemd-oomd killed the desktop session.

This is the same bug as #473, which was diagnosed and closed in 2023 without a code change. Filing again with a mechanism-level analysis and measurements, because the mitigation applied then (forbidding build.delay = 0) cannot address it.

Verified against master (9f19e5251, == v1.67.4) and v1.65.1; the relevant code is identical in both.

Mechanism

Two independent problems combine.

1. building() is not cancellable. runCommandCopyOutput is a blocking cmd.Wait() with no context and no kill path, and myStopCh is never plumbed past its call site:

func (e *Engine) buildRun() {
	myStopCh := make(chan struct{})
	e.buildRunCh <- myStopCh
	defer func() { <-e.buildRunCh }()

	select { case <-myStopCh: return; ... default: }   // checked BEFORE
	...
	if output, err := e.building(); err != nil { ... } //  <-- not cancellable
	select { case <-myStopCh: return; ... default: }   // checked AFTER

Closing myStopCh therefore stops nothing that is already compiling. It only prevents the finished build from launching its binary.

2. The buildRunCh "semaphore" gates nothing. The field is documented as "acts as semaphore + carries our stop token" and "ensuring only one build runs at a time (buffer size 1)", but the main loop drains the buffer one statement before spawning the next build:

select {
case oldStopCh := <-e.buildRunCh:
    close(oldStopCh)
default:
}
e.stopBinBeforeBuildIfNeeded(runtime.GOOS)
go e.buildRun()          // spawned unconditionally, every event

So e.buildRunCh <- myStopCh always succeeds immediately and the cap-1 buffer never blocks anything.

Worse, the defer performs an untyped receive: it drains whatever token is present, not necessarily its own. A finishing build can therefore consume its successor's token, after which the main loop's non-blocking receive hits default and the successor's stop channel is never closed. From the third consecutive rebuild onward, a superseded build reaches runBin() and launches a stale binary while a newer build is still running — two runBin goroutines then race the binStopCh assignment, which can orphan the app process holding the port.

Reproduction

Any project where a build takes longer than the interval between saves. Ours is a 1.24 M-line monolith at ~14 s cold / ~2-4 s incremental.

1. air with a build slower than build.delay.
2. Save a watched file repeatedly, a couple of seconds apart.
3. pgrep -fc 'go build' climbs without bound; so does ls -d $TMPDIR/go-build* | wc -l.

Why build.delay cannot fix this

The debounce coalesces a burst of events. It does nothing for a save arriving 5 s into a 30 s build, which is the actual failure. Stacking requires only build_time > save_interval.

Secondary effect: leaked build work dirs

Each cancelled go build orphans its work dir (~56 MB here). cmd/go installs the signal handler that runs base.AtExit(closeBuilders) → RemoveAll(b.WorkDir) only for go run and go test, never for go build, so no signal lets a killed go build clean up. On distributions where /tmp is tmpfs this consumes RAM: our 45 orphans were ~2.5 GB of it, on top of the compiler memory.

This is a consequence of adding cancellation, not a bug in air today (air never cancels anything, so it cannot leak). Worth designing for: a cancelling implementation should give each build its own GOTMPDIR and remove it after the kill.

Related: #932 (build output accumulated via unbounded io.ReadAll) multiplies the blast radius — 45 concurrent builds also meant 45 unbounded stdout buffers.

Suggested direction

The repo already contains the shape of the fix. runner/rule.go's runRule is a serialized single-worker loop — sleep the delay, drain to coalesce, run synchronously — which cannot stack. Applying that shape to buildRun, plus threading a context.Context from myStopCh into startCmd via exec.CommandContext with Cmd.Cancel signalling the process group (SysProcAttr.Setpgid: true is already set) and Cmd.WaitDelay for escalation, would close both halves. Fixing the untyped defer receive is worthwhile independently of cancellation.

Workaround, for anyone hitting this

We wrap [build] cmd in a script that records its own process group, cancels the group recorded by the previous invocation, and only then runs go build — newest build wins, at most one in flight. Three details that were not obvious: kill -0 -- -<pgid> succeeds on a group whose only member is a zombie, so liveness must be decided some other way; a pid-recycling guard is mandatory before signalling a stored pgid; and the state file must be claimed before the predecessor is cancelled, or a save arriving during the cancellation starts a third build.