Don't let a leaked subprocess wedge the headless test runner

When lazygit exits but leaves behind a subprocess that inherited its
stderr pipe and has detached from the pty, cmd.Wait() blocks in
awaitGoroutines waiting for that pipe to reach EOF -- which never
happens while the straggler is alive. With no WaitDelay set, that wait
is unbounded, so a single leaked process hangs the whole test binary
until the 10-minute global timeout fires and panics. Worse, the timeout
discards whatever lazygit wrote to stderr before exiting (a panic, a
-race report), which is exactly the output needed to diagnose the
failure.

This surfaces under -race, where lazygit runs slow enough to widen the
window for a spawned command to still be alive when lazygit quits, and
it's a blocker for enabling the race detector on CI.

Bound the wait with cmd.WaitDelay so Wait force-closes the pipe and
returns ErrWaitDelay instead of hanging, surface the captured stderr as
the error (falling back to the wait error when nothing was printed), and
kill the child's process group on failure so a straggler can't linger
into a later test or pile up across a run.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Stefan Haller 2026-07-09 19:11:33 +02:00
parent 4390e1fb08
commit 46c1fa7db2

View file

@ -11,7 +11,9 @@ import (
"io"
"os"
"os/exec"
"syscall"
"testing"
"time"
"github.com/creack/pty"
"github.com/jesseduffield/lazycore/pkg/utils"
@ -75,6 +77,17 @@ func runCmdHeadless(cmd *exec.Cmd) (int, error) {
stderr := new(bytes.Buffer)
cmd.Stderr = stderr
// If lazygit exits but leaves behind a subprocess that inherited its stderr
// pipe, cmd.Wait blocks waiting for that pipe to reach EOF for as long as the
// subprocess stays alive. Unbounded, that hangs the whole test binary until
// its global timeout fires, and the timeout throws away whatever lazygit
// wrote to stderr before exiting (a panic, a -race report) -- the very output
// needed to diagnose the failure. WaitDelay caps the wait: once the process
// has exited, Wait gives the stderr goroutine at most this long to drain,
// then closes the pipe and returns ErrWaitDelay, so the captured stderr
// surfaces as the test error instead of being lost.
cmd.WaitDelay = 5 * time.Second
// these rows and columns are ignored because internally we use tcell's
// simulation screen. However we still need the pty for the sake of
// running other commands in a pty.
@ -83,12 +96,32 @@ func runCmdHeadless(cmd *exec.Cmd) (int, error) {
return -1, err
}
// pty.StartWithSize starts lazygit in its own process group, so we can signal
// the whole group at once. Capture the id now, while the process is alive:
// once Wait has reaped it we can no longer look it up.
pgid, pgidErr := syscall.Getpgid(cmd.Process.Pid)
_, _ = io.Copy(io.Discard, f)
if cmd.Wait() != nil {
waitErr := cmd.Wait()
// On any failure -- including a WaitDelay expiry caused by a leaked
// subprocess -- kill the whole process group so a straggler can't linger and
// wedge a later test or pile up across a CI run. Best effort: usually the
// group is already gone (ESRCH), and a subprocess that called setsid to
// detach into its own group is out of reach, but WaitDelay still unblocks us.
if waitErr != nil && pgidErr == nil {
_ = syscall.Kill(-pgid, syscall.SIGKILL)
}
if waitErr != nil {
_ = f.Close()
// return an error with the stderr output
return cmd.Process.Pid, errors.New(stderr.String())
// Prefer lazygit's own stderr as the error; fall back to the wait error
// itself (e.g. ErrWaitDelay) when it exited without printing anything.
if stderr.Len() > 0 {
return cmd.Process.Pid, errors.New(stderr.String())
}
return cmd.Process.Pid, waitErr
}
return cmd.Process.Pid, f.Close()