From 73ca87d19ef8e1a637ae3ec32079128c26889948 Mon Sep 17 00:00:00 2001 From: Marcus Messer Date: Thu, 20 Aug 2026 17:52:02 +0100 Subject: [PATCH] Add execution timeout handling for evaluation and preview functions; update tests for timeout scenarios. --- Kernel/EvaluationFunctionToolkit/Execution.wl | 45 ++++++++++- Tests/Execution.wlt | 75 +++++++++++++++++++ 2 files changed, 118 insertions(+), 2 deletions(-) create mode 100644 Tests/Execution.wlt diff --git a/Kernel/EvaluationFunctionToolkit/Execution.wl b/Kernel/EvaluationFunctionToolkit/Execution.wl index d99e052..1f73589 100644 --- a/Kernel/EvaluationFunctionToolkit/Execution.wl +++ b/Kernel/EvaluationFunctionToolkit/Execution.wl @@ -5,13 +5,50 @@ normalized outcome association (`"ok" -> True/False`), independent of how the calling transport eventually formats that outcome on the wire. *) +(* Bound (seconds) on how long a single eval/preview call may run, before + shimmy's own RPC-level timeout would otherwise give up on it and leave + the (persistent, shared) dedicated worker wedged on the still-running + call for every later request -- shimmy never recycles a worker after a + Send timeout, so a runaway computation (e.g. Simplify/FullSimplify on an + expression with free transcendental parameters, or pathological pattern + matching) must abort inside the kernel first. + + Derived from shimmy's own default worker-send-timeout (30s -- see + FUNCTION_WORKER_SEND_TIMEOUT / "worker-send-timeout" in shimmy's + cmd/root.go) minus a safety buffer, rather than picked independently, so + the two timeouts can't silently drift apart. *) +$shimmyDefaultSendTimeout = 30; +$executionTimeoutBuffer = 5; +$defaultExecutionTimeout = $shimmyDefaultSendTimeout - $executionTimeoutBuffer; + +(* Reads EVAL_EXECUTION_TIMEOUT (seconds) as an operator-facing override, + mirroring the Environment[...] reading pattern in Dispatch.wl's + resolveDispatchTarget. Falls back to $defaultExecutionTimeout for + anything unset or not a positive number. *) +executionTimeout[] := Module[{raw, parsed}, + raw = Environment["EVAL_EXECUTION_TIMEOUT"]; + If[raw === $Failed || raw === "", Return[$defaultExecutionTimeout]]; + + parsed = Quiet@Check[ToExpression[raw], $Failed]; + If[NumericQ[parsed] && parsed > 0, parsed, $defaultExecutionTimeout] +]; + (* Catches Wolfram Messages raised by user code so a crash still produces a - normalized failure outcome instead of propagating. *) -safeCall[fn_, args___] := Quiet@Check[fn[args], $Failed]; + normalized failure outcome instead of propagating, and bounds execution + time so a runaway computation can't hang the (persistent, shared) kernel + indefinitely. *) +safeCall[fn_, args___] := Quiet@Check[ + TimeConstrained[fn[args], executionTimeout[], $TimedOut], + $Failed +]; runEval[evalFn_, answer_, response_, params_] := Module[{result, errorMsg}, result = safeCall[evalFn, answer, response, params]; + If[result === $TimedOut, + Return[<| "ok" -> False, "message" -> "Evaluation function timed out" |>] + ]; + If[result === $Failed, Return[<| "ok" -> False, "message" -> "Evaluation function raised an error" |>] ]; @@ -30,6 +67,10 @@ runEval[evalFn_, answer_, response_, params_] := Module[{result, errorMsg}, runPreview[previewFn_, response_, params_] := Module[{result}, result = safeCall[previewFn, response, params]; + If[result === $TimedOut, + Return[<| "ok" -> False, "message" -> "Preview function timed out" |>] + ]; + If[result === $Failed, Return[<| "ok" -> False, "message" -> "Preview function raised an error" |>] ]; diff --git a/Tests/Execution.wlt b/Tests/Execution.wlt new file mode 100644 index 0000000..7ac585c --- /dev/null +++ b/Tests/Execution.wlt @@ -0,0 +1,75 @@ +(* ::Package:: *) + +Needs["LambdaFeedback`EvaluationFunctionToolkit`"] + +runEval = LambdaFeedback`EvaluationFunctionToolkit`Private`runEval; +runPreview = LambdaFeedback`EvaluationFunctionToolkit`Private`runPreview; + +(* Tests/*.wlt run in one shared kernel session per build-and-test.yml, so + restore EVAL_EXECUTION_TIMEOUT afterward. Set well below the real default + so timeout cases don't slow the suite down. *) +withExecutionTimeout[seconds_, testFn_] := Module[{saved, result}, + saved = Environment["EVAL_EXECUTION_TIMEOUT"]; + SetEnvironment["EVAL_EXECUTION_TIMEOUT" -> ToString[seconds]]; + result = testFn[]; + SetEnvironment["EVAL_EXECUTION_TIMEOUT" -> If[saved === $Failed, "", saved]]; + result +]; + +fastEvalFn[answer_, response_, params_] := <| + "error" -> Null, + "is_correct" -> TrueQ[answer == response] +|>; + +(* 1/0 reliably raises a genuine Wolfram Message (Power::infy) for Check to + catch, matching how a careless user eval function might fail. *) +erroringEvalFn[answer_, response_, params_] := 1/0; + +hangingEvalFn[answer_, response_, params_] := (Pause[10]; <| + "error" -> Null, + "is_correct" -> True +|>); + +fastPreviewFn[response_, params_] := <| "latex" -> response, "sympy" -> response |>; + +hangingPreviewFn[response_, params_] := (Pause[10]; <| "latex" -> response, "sympy" -> response |>); + +VerificationTest[ + runEval[fastEvalFn, "1", "1", <||>]["ok"], + True, + TestID -> "Execution-runEval-fast-call-succeeds" +] + +VerificationTest[ + runEval[erroringEvalFn, "1", "1", <||>], + <| "ok" -> False, "message" -> "Evaluation function raised an error" |>, + TestID -> "Execution-runEval-message-still-caught-as-error" +] + +VerificationTest[ + withExecutionTimeout[2, runEval[hangingEvalFn, "1", "1", <||>] &], + <| "ok" -> False, "message" -> "Evaluation function timed out" |>, + TestID -> "Execution-runEval-timeout" +] + +VerificationTest[ + runPreview[fastPreviewFn, "x+1", <||>]["ok"], + True, + TestID -> "Execution-runPreview-fast-call-succeeds" +] + +VerificationTest[ + withExecutionTimeout[2, runPreview[hangingPreviewFn, "x+1", <||>] &], + <| "ok" -> False, "message" -> "Preview function timed out" |>, + TestID -> "Execution-runPreview-timeout" +] + +(* Regression: the timeout must actually bound wall-clock time, not just + the eventual outcome -- guards against a future change accidentally + dropping TimeConstrained while still returning the right-shaped result + some other way. *) +VerificationTest[ + First[withExecutionTimeout[2, AbsoluteTiming[runEval[hangingEvalFn, "1", "1", <||>]] &]] < 9, + True, + TestID -> "Execution-runEval-timeout-bounds-wall-clock-time" +] \ No newline at end of file