Add execution timeout handling for evaluation and preview functions; …

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" |>]
   ];

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"
+]