From f599d85f961603623fff33c67a54bba7ec6d7b73 Mon Sep 17 00:00:00 2001 From: AJ Stuyvenberg Date: Sat, 31 Oct 2026 18:01:29 -0400 Subject: [PATCH 1/2] fix(interp): end the run when a builtin returns after its deadline passed `read` installs the run's deadline as its own kernel read deadline, so it can wake and return status 142 just before the context timer sets ctx.Err(). The post-builtin check only looked at ctx.Err(), so the script carried on and Run returned nil: a run that hit its deadline reported success. The check now also treats a passed ctx.Deadline() as expired and stops the run with context.DeadlineExceeded, for every builtin. It waits up to 100ms for the context timer to publish ctx.Err(), so callers that check the context after Run returns see it cancelled. Found by Jacob Hayes with patina (https://github.com/JacobHayes/patina). Environment: Datadog workspace Co-Authored-By: Claude Opus 5.5 --- interp/runner_exec.go | 19 +++++++++++++++++++ interp/timeout_test.go | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 52 insertions(+) diff --git a/interp/runner_exec.go b/interp/runner_exec.go index 92e0fab16..30849cef7 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 fc9b4a04b..a0e124746 100644 --- a/interp/timeout_test.go +++ b/interp/timeout_test.go @@ -6,7 +6,9 @@ package interp import ( + "bytes" "context" + "os" "testing" "time" @@ -108,3 +110,34 @@ 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) { + 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") +} From 3f8093836c3f6d7d0230d670ffaadc35e1049931 Mon Sep 17 00:00:00 2001 From: AJ Stuyvenberg Date: Sat, 31 Oct 2026 20:04:30 -0400 Subject: [PATCH 2/2] test(interp): skip the read deadline race test on Windows Windows pipes have no read deadlines, so read waits on ctx.Done() there and cannot race ahead of the context; the test context never fires Done. Environment: Datadog workspace Co-Authored-By: Claude Opus 5.5 --- interp/timeout_test.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/interp/timeout_test.go b/interp/timeout_test.go index a0e124746..cdaf704c8 100644 --- a/interp/timeout_test.go +++ b/interp/timeout_test.go @@ -9,6 +9,7 @@ import ( "bytes" "context" "os" + "runtime" "testing" "time" @@ -125,6 +126,11 @@ func (c raceCtx) Deadline() (time.Time, bool) { return c.deadline, true } // 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() })