diff --git a/interp/runner_exec.go b/interp/runner_exec.go index 92e0fab1..30849cef 100644 --- a/interp/runner_exec.go +++ b/interp/runner_exec.go @@ -1228,8 +1228,24 @@ func (r *Runner) call(ctx context.Context, pos syntax.Pos, args []string, setup // builtin's status (e.g. `read -t`'s 142) instead of the // underlying context.DeadlineExceeded that cmd/rshell main checks // for to print "execution timed out". + // + // A builtin may also turn the run's deadline into its own timeout + // (`read` installs it as a kernel read deadline) and return just + // before the context's timer sets ctx.Err(). A deadline that has + // passed counts as expired, so the run still ends with the + // deadline error instead of the script carrying on. if err := ctx.Err(); err != nil { r.exit.fatal(err) + } else if dl, ok := ctx.Deadline(); ok && !time.Now().Before(dl) { + // Give the timer a moment to publish ctx.Err() so callers + // that check it after Run returns see a cancelled context. + graceCtx, cancelGrace := context.WithDeadline(context.Background(), time.Now().Add(deadlineGrace)) + select { + case <-ctx.Done(): + case <-graceCtx.Done(): + } + cancelGrace() + r.exit.fatal(context.DeadlineExceeded) } return } @@ -1238,6 +1254,9 @@ func (r *Runner) call(ctx context.Context, pos syntax.Pos, args []string, setup r.exec(ctx, pos, args) } +// deadlineGrace bounds how long a passed deadline waits for its context timer. +const deadlineGrace = time.Second / 10 + func (r *Runner) exec(ctx context.Context, pos syntax.Pos, args []string) { r.exit.fromHandlerError(r.execHandler(r.handlerCtx(ctx, pos), args)) } diff --git a/interp/timeout_test.go b/interp/timeout_test.go index fc9b4a04..cdaf704c 100644 --- a/interp/timeout_test.go +++ b/interp/timeout_test.go @@ -6,7 +6,10 @@ package interp import ( + "bytes" "context" + "os" + "runtime" "testing" "time" @@ -108,3 +111,39 @@ func TestMaxExecutionTimeIsRefreshedPerRun(t *testing.T) { require.Len(t, deadlines, 2) assert.True(t, deadlines[1].After(deadlines[0]), "expected a fresh deadline on each Run") } + +// raceCtx reports a deadline but never fires: it reproduces the window where a +// builtin's own kernel read deadline (derived from ctx.Deadline) expires before +// the context's timer goroutine has set ctx.Err(). +type raceCtx struct { + context.Context + deadline time.Time +} + +func (c raceCtx) Deadline() (time.Time, bool) { return c.deadline, true } + +// TestReadHittingRunDeadlineIsNotSuccess guards against a run that hit its +// deadline reporting success: read turns the run deadline into its own +// status-142 timeout, and the script must neither continue nor return nil. +func TestReadHittingRunDeadlineIsNotSuccess(t *testing.T) { + if runtime.GOOS == "windows" { + // Windows pipes have no read deadlines, so read waits on ctx.Done() + // there and cannot race ahead of it; this ctx never fires Done. + t.Skip("read cannot install a kernel read deadline on Windows pipes") + } + pr, pw, err := os.Pipe() + require.NoError(t, err) + t.Cleanup(func() { _ = pr.Close(); _ = pw.Close() }) + + var out bytes.Buffer + r := newTimeoutRunner(t, StdIO(pr, &out, &out)) + + parent, cancel := context.WithCancel(context.Background()) + defer cancel() + ctx := raceCtx{Context: parent, deadline: time.Now().Add(20 * time.Millisecond)} + + err = r.Run(ctx, parseScript(t, "read x; echo after")) + require.Error(t, err) + assert.ErrorIs(t, err, context.DeadlineExceeded) + assert.NotContains(t, out.String(), "after") +}