fix(scripting): stop unbounded script recursion from killing the server (lmxopcua#779)
Reproduced first, out of process (ScriptRecursionContainmentTests re-launches the xunit v3 test assembly to run one recursive script through ScriptEvaluator + ScriptRunGuard, so the runner never executes one): before this change every shape — local function, mutual local functions, self-referencing Func<>, anonymous self-application, recursion through LINQ, an expression-bodied void lambda, an anonymous method — ended the child with exit 134 (SIGABRT, "Stack overflow."). A StackOverflowException cannot be caught, so in production one script takes the whole node down. Fix: ScriptEvaluator.Compile now rewrites the validated wrapper (ScriptStackGuardInjector, after ForbiddenTypeAnalyzer) so the script body and every lambda, anonymous method and local function starts with ScriptStackGuard.Check(), which throws ScriptStackExhaustedException when RuntimeHelpers.TryEnsureSufficientExecutionStack reports the stack nearly spent. The script then fails like any throwing script (Bad, script log) and the process lives. Why a runtime check, not an analyzer rule: recursion is not decidable from syntax (anonymous self-application never names itself), and bounded recursion must keep working. Expression-bodied lambdas/local functions become blocks (`return e;` or `e;`, chosen from the bound symbol, so the same overload binds); expression-tree and async lambdas are left alone. Limits (documented on ScriptStackGuard and in docs/VirtualTags.md): depth depends on the thread's stack size; a recursion that never re-enters script code is not seen; a script may catch the exception, which only unwinds it. Tests: ScriptRecursionContainmentTests (7 shapes, child process each; all 7 fail with exit 134 when the rewrite is disabled), ScriptStackGuardInjectionTests (bounded recursion and every rewritten lambda/local-function shape keep their value; the expression-tree case fails if those lambdas are rewritten). Claude-Session: https://claude.ai/code/session_01GT67AEHd2Acv1YPwGfXEoV
This commit is contained in:
Joseph Doherty
@@ -0,0 +1,141 @@
|
||||
using System.Diagnostics;
|
||||
using Shouldly;
|
||||
using Xunit;
|
||||
|
||||
namespace ZB.MOM.WW.OtOpcUa.Core.Scripting.Tests;
|
||||
|
||||
/// <summary>
|
||||
/// lmxopcua#779 — can unbounded recursion in a user script take the whole host process down? A
|
||||
/// <see cref="StackOverflowException"/> cannot be caught in .NET, so the only safe way to ask is in a
|
||||
/// <b>separate process</b>: each case re-launches this test assembly (an xunit v3 executable) to run exactly one
|
||||
/// <see cref="Child_runs_one_recursive_script_through_the_production_run_path"/> case, and inspects how that
|
||||
/// process ended. The test runner itself never executes a recursive script.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// Before the guard every case below ended the child with "Stack overflow." and SIGABRT (exit 134 on macOS) — the
|
||||
/// reproduction recorded in the #779 commit. Now the script fails with
|
||||
/// <see cref="ScriptStackExhaustedException"/>, which the run path reports as an ordinary script fault, and the
|
||||
/// process lives on.
|
||||
/// </remarks>
|
||||
public sealed class ScriptRecursionContainmentTests
|
||||
{
|
||||
private const string ScenarioVariable = "OTOPCUA_RECURSION_CHILD_SCENARIO";
|
||||
private const string ResultFileVariable = "OTOPCUA_RECURSION_CHILD_RESULT";
|
||||
|
||||
/// <summary>Recursive shapes a script can write. None of them is a class member, so the wrapper's
|
||||
/// single-member check never sees them; the last two have no named self-reference at all.</summary>
|
||||
public static readonly Dictionary<string, string> Scenarios = new(StringComparer.Ordinal)
|
||||
{
|
||||
["local-function"] =
|
||||
"int Down(int n) => Down(n + 1) + 1; return Down(0);",
|
||||
["mutual-local-functions"] =
|
||||
"int Ping(int n) { return Pong(n + 1) + 1; } int Pong(int n) { return Ping(n + 1) + 1; } return Ping(0);",
|
||||
["self-referencing-lambda"] =
|
||||
"System.Func<int, int> f = null!; f = n => f(n + 1) + 1; return f(0);",
|
||||
["anonymous-self-application"] =
|
||||
"System.Func<object, int> g = s => ((System.Func<object, int>)s)(s) + 1; return g(g);",
|
||||
["recursion-through-linq"] =
|
||||
"System.Func<int, int> f = null!; f = n => System.Linq.Enumerable.Sum(new[] { n }, x => f(x + 1)) + 1; return f(0);",
|
||||
// An expression-bodied VOID lambda (rewritten to `{ Check(); e; }`), recursing through a value lambda. A
|
||||
// bare `a = n => a(n + 1)` is no test: the JIT turns that tail call into a loop, which never overflows and
|
||||
// is the timeout's business.
|
||||
["void-lambda"] =
|
||||
"System.Action<int> a = null!; System.Func<int, int> h = n => { a(n); return n; }; a = n => h(n + 1); a(0); return 0;",
|
||||
["anonymous-method"] =
|
||||
"System.Func<int, int> f = null!; f = delegate (int n) { return f(n + 1) + 1; }; return f(0);",
|
||||
};
|
||||
|
||||
public static TheoryData<string> ScenarioNames => new(Scenarios.Keys);
|
||||
|
||||
[Theory]
|
||||
[MemberData(nameof(ScenarioNames))]
|
||||
public async Task A_recursive_script_fails_the_script_and_does_not_kill_the_process(string scenario)
|
||||
{
|
||||
var resultFile = Path.Combine(Path.GetTempPath(), $"otopcua-recursion-{Guid.NewGuid():N}.txt");
|
||||
try
|
||||
{
|
||||
var (exitCode, stdout, stderr) = await RunChildAsync(scenario, resultFile);
|
||||
var outcome = File.Exists(resultFile) ? await File.ReadAllTextAsync(resultFile, TestContext.Current.CancellationToken) : "<no result file>";
|
||||
|
||||
// Assert the artifact first: what the child itself recorded about the run.
|
||||
outcome.ShouldBe(
|
||||
$"FAULTED:{nameof(ScriptStackExhaustedException)}",
|
||||
$"child exit {exitCode}; stderr head: {Head(stderr)}; stderr tail: {Tail(stderr)}; stdout tail: {Tail(stdout)}");
|
||||
exitCode.ShouldBe(0, $"the process must survive the script; stderr tail: {Tail(stderr)}");
|
||||
}
|
||||
finally
|
||||
{
|
||||
File.Delete(resultFile);
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// The child half. Skipped in a normal run; it executes only inside a process launched by
|
||||
/// <see cref="RunChildAsync"/>, where it compiles one scenario and runs it the way the production seams do
|
||||
/// (<see cref="ScriptRunGuard"/> on the thread pool), then records how the run ended.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public async Task Child_runs_one_recursive_script_through_the_production_run_path()
|
||||
{
|
||||
var scenario = Environment.GetEnvironmentVariable(ScenarioVariable);
|
||||
var resultFile = Environment.GetEnvironmentVariable(ResultFileVariable);
|
||||
if (string.IsNullOrEmpty(scenario) || string.IsNullOrEmpty(resultFile))
|
||||
Assert.Skip("Child half of ScriptRecursionContainmentTests; runs only in the process that test launches.");
|
||||
|
||||
var source = Scenarios[scenario];
|
||||
using var evaluator = ScriptEvaluator<FakeScriptContext, int>.Compile(source);
|
||||
var guard = new ScriptRunGuard();
|
||||
string outcome;
|
||||
try
|
||||
{
|
||||
var value = await guard.RunAsync(scenario, source, evaluator, new FakeScriptContext(), TimeSpan.FromSeconds(60), TestContext.Current.CancellationToken);
|
||||
outcome = $"RETURNED:{value}";
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
outcome = $"FAULTED:{ex.GetType().Name}";
|
||||
}
|
||||
|
||||
await File.WriteAllTextAsync(resultFile, outcome, TestContext.Current.CancellationToken);
|
||||
}
|
||||
|
||||
private static async Task<(int ExitCode, string Stdout, string Stderr)> RunChildAsync(string scenario, string resultFile)
|
||||
{
|
||||
var assembly = typeof(ScriptRecursionContainmentTests).Assembly.Location;
|
||||
var childTest = $"{typeof(ScriptRecursionContainmentTests).FullName}.{nameof(Child_runs_one_recursive_script_through_the_production_run_path)}";
|
||||
var psi = new ProcessStartInfo("dotnet")
|
||||
{
|
||||
RedirectStandardOutput = true,
|
||||
RedirectStandardError = true,
|
||||
UseShellExecute = false,
|
||||
};
|
||||
psi.ArgumentList.Add("exec");
|
||||
psi.ArgumentList.Add(assembly);
|
||||
psi.ArgumentList.Add("-method");
|
||||
psi.ArgumentList.Add(childTest);
|
||||
psi.Environment[ScenarioVariable] = scenario;
|
||||
psi.Environment[ResultFileVariable] = resultFile;
|
||||
// A crash dump of a deliberate stack overflow is noise, not evidence.
|
||||
psi.Environment["DOTNET_DbgEnableMiniDump"] = "0";
|
||||
|
||||
using var process = Process.Start(psi) ?? throw new InvalidOperationException("could not start the child test process");
|
||||
var stdout = process.StandardOutput.ReadToEndAsync();
|
||||
var stderr = process.StandardError.ReadToEndAsync();
|
||||
using var cts = new CancellationTokenSource(TimeSpan.FromMinutes(2));
|
||||
try
|
||||
{
|
||||
await process.WaitForExitAsync(cts.Token);
|
||||
}
|
||||
catch (OperationCanceledException)
|
||||
{
|
||||
process.Kill(entireProcessTree: true);
|
||||
throw new TimeoutException($"child for '{scenario}' did not exit within 2 minutes");
|
||||
}
|
||||
|
||||
return (process.ExitCode, await stdout, await stderr);
|
||||
}
|
||||
|
||||
private static string Head(string text) => text.Length <= 200 ? text : text[..200];
|
||||
|
||||
private static string Tail(string text) => text.Length <= 600 ? text : text[^600..];
|
||||
}
|
||||
Reference in New Issue
Block a user