commit ab99185806a086f2ce26c222dbc59384f051f87c
parent d33392d0fd7c0fa984698cd6670b5081a8be247f
Author: Jack Mordaunt <jackmordaunt.dev@gmail.com>
Date: Fri, 18 Sep 2026 13:15:42 -0400
audio: bound the teardown of an abandoned ffmpeg stream
Closing a stream that was not drained killed the process and then waited
for it, which assumed the kill worked. On a runner whose ffmpeg comes
through a launcher shim it does not: killing the shim leaves the real
process alive holding the pipe, so the wait never returned and Close
hung.
Teardown now has a deadline. Whatever still holds the pipe finishes
decoding shortly and exits by itself, so this is a brief leak rather
than a caller that cannot return. Draining the pipe before waiting also
observes the contract the standard library documents, which the previous
version quietly broke.
Diffstat:
1 file changed, 34 insertions(+), 7 deletions(-)
diff --git a/audio_ffmpeg.go b/audio_ffmpeg.go
@@ -17,6 +17,7 @@ import (
"strconv"
"strings"
"sync"
+ "time"
)
// FFmpegLoad raw PCM with ffmpeg.
@@ -176,16 +177,42 @@ func (f *ffmpegStream) Read(p []byte) (int, error) {
return n, err
}
+// reapTimeout bounds how long Close waits for an abandoned ffmpeg to go
+// away before leaving it to finish on its own.
+const reapTimeout = 5 * time.Second
+
func (f *ffmpegStream) Close() error {
- if !f.drained {
- // Abandoned early. Kill it rather than leave ffmpeg blocked
- // writing into a pipe nobody is reading, then reap the corpse.
- // The resulting wait error is ours, not a decode failure.
- _ = f.cmd.Process.Kill()
+ if f.drained {
+ return f.reap()
+ }
+
+ // Abandoned early. Kill it rather than leave ffmpeg blocked writing
+ // into a pipe nobody is reading.
+ _ = f.cmd.Process.Kill()
+
+ // Waiting on a command wants its pipes drained first, and the wait
+ // itself only returns once nothing holds the far end. Neither is
+ // guaranteed when ffmpeg is reached through a launcher shim, since
+ // killing the shim leaves the real process running and holding the
+ // pipe. CI found exactly that: closing an abandoned stream never
+ // returned on a runner whose ffmpeg came from a package manager that
+ // installs one.
+ //
+ // So the teardown gets a deadline. Whatever is still holding the pipe
+ // finishes decoding shortly and exits on its own, which makes this a
+ // brief leak rather than a caller that never returns.
+ done := make(chan struct{})
+ go func() {
+ defer close(done)
+ _, _ = io.Copy(io.Discard, f.stdout)
_ = f.reap()
- return nil
+ }()
+
+ select {
+ case <-done:
+ case <-time.After(reapTimeout):
}
- return f.reap()
+ return nil
}
// FFmpegStream decodes an audio file with ffmpeg, returning PCM through
