Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 18 additions & 1 deletion internal/process/process.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,15 @@ const termType = "xterm-256color"
// fires before os/exec's WaitDelay-triggered kill, keeping cancellation
// behaviour unchanged, while still bounding the post-exit I/O wait so a
// leaked stdout/stderr pipe can never hang Cmd.Wait indefinitely.
const waitDelayBuffer = 1 * time.Second
//
// The bound is deliberately generous: when it expires, os/exec force-closes
// the output pipes and any not-yet-drained output is silently lost (Wait
// returns ErrWaitDelay, which complete() treats as a clean exit). A 1s buffer
// proved too tight on loaded Windows CI machines, where the post-exit drain of
// fast hooks intermittently exceeded it and hook output went missing. Hitting
// this bound should only ever mean a genuinely leaked pipe write-end, not a
// slow-but-healthy drain.
const waitDelayBuffer = 30 * time.Second

// afterPTYStartHook lets tests force work to happen after the PTY helper
// returns so they can verify raw-mode ordering around process startup.
Expand Down Expand Up @@ -99,6 +107,12 @@ type Config struct {
SignalGracePeriod time.Duration
Started chan struct{}
Done chan struct{}

// WaitDelay, when positive, overrides the derived Cmd.WaitDelay bound
// (SignalGracePeriod + waitDelayBuffer) for the non-PTY path. Production
// code leaves this zero; tests use it to keep leaked-pipe regression
// tests fast.
WaitDelay time.Duration
}

// Process is an operating system level process
Expand Down Expand Up @@ -315,6 +329,9 @@ func (p *Process) startWithoutPTY(context.Context) (func(), error) {
// cancellation; this keeps cancellation behaviour identical to before while
// still bounding the post-exit I/O wait for the leaked-pipe case.
func (p *Process) waitDelay() time.Duration {
if p.conf.WaitDelay > 0 {
return p.conf.WaitDelay
}
return max(p.conf.SignalGracePeriod, 0) + waitDelayBuffer
}

Expand Down
2 changes: 2 additions & 0 deletions internal/process/process_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -460,6 +460,8 @@ func TestProcessRunDoesNotHangWhenChildLeaksStdout(t *testing.T) {
Env: []string{"TEST_MAIN=leak-stdout"},
Stdout: w,
Stderr: w,
// Shorten the post-exit I/O bound (default 30s) so the test stays fast.
WaitDelay: time.Second,
})

// Ensure the leaked grandchild is cleaned up regardless of outcome.
Expand Down
6 changes: 6 additions & 0 deletions internal/shell/export_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,9 @@ import "time"
func Round(d time.Duration) time.Duration {
return round(d)
}

// WithProcessWaitDelay is a test-only shell option that overrides the derived
// process.Config.WaitDelay bound, keeping leaked-pipe regression tests fast.
func WithProcessWaitDelay(d time.Duration) NewShellOpt {
return func(s *Shell) { s.processWaitDelay = d }
}
6 changes: 6 additions & 0 deletions internal/shell/shell.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,10 @@ type Shell struct {
// Amount of time to wait between sending the InterruptSignal and SIGKILL
signalGracePeriod time.Duration

// Test-only override for the derived process.Config.WaitDelay bound
// (see WithProcessWaitDelay in export_test.go). Zero means "use default".
processWaitDelay time.Duration

// stdin is an optional input stream used by Run() and friends.
// It remains unexported on the assumption that it's not useful except via
// CloneWithStdin to get a clone prepared for a single command that needs
Expand Down Expand Up @@ -151,6 +155,7 @@ func (s *Shell) CloneWithStdin(r io.Reader) *Shell {
wd: s.wd,
interruptSignal: s.interruptSignal,
signalGracePeriod: s.signalGracePeriod,
processWaitDelay: s.processWaitDelay,
}
}

Expand Down Expand Up @@ -576,6 +581,7 @@ func (s *Shell) buildCommand(name string, arg ...string) (process.Config, error)
Dir: s.wd,
InterruptSignal: s.interruptSignal,
SignalGracePeriod: s.signalGracePeriod,
WaitDelay: s.processWaitDelay,
}, nil
}

Expand Down
2 changes: 2 additions & 0 deletions internal/shell/shell_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -757,6 +757,8 @@ func TestRunDoesNotReportCleanHookAsFailedWhenChildLeaksStdout(t *testing.T) {
sh, err := shell.New(
shell.WithStdout(w),
shell.WithLogger(shell.DiscardLogger),
// Shorten the post-exit I/O bound (default 30s) so the test stays fast.
shell.WithProcessWaitDelay(time.Second),
)
if err != nil {
t.Fatalf("shell.New() error = %v", err)
Expand Down