From ef00d6e5aafeecaf34fe1d6c7da786a457f6fba0 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 13 Jul 2026 10:12:59 -0400 Subject: [PATCH] fix(host): retry VT evaluation once on ObjectDisposedException from the apply-boundary cache clear (02/S12) --- ...2-03-vt-failure-quality-plan.md.tasks.json | 2 +- .../Engines/RoslynVirtualTagEvaluator.cs | 116 ++++++++++++------ 2 files changed, 78 insertions(+), 40 deletions(-) diff --git a/archreview/plans/R2-03-vt-failure-quality-plan.md.tasks.json b/archreview/plans/R2-03-vt-failure-quality-plan.md.tasks.json index 35acaa3d..595d60b5 100644 --- a/archreview/plans/R2-03-vt-failure-quality-plan.md.tasks.json +++ b/archreview/plans/R2-03-vt-failure-quality-plan.md.tasks.json @@ -54,7 +54,7 @@ { "id": "R2-03-T8", "subject": "S12 fix: retry-once on ObjectDisposedException in RoslynVirtualTagEvaluator.Evaluate (re-fetch via GetOrCompile; no retry when _disposed or second hit); update xmldoc (turns T7 green)", - "status": "pending", + "status": "completed", "blockedBy": [ "R2-03-T6", "R2-03-T7" diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Engines/RoslynVirtualTagEvaluator.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Engines/RoslynVirtualTagEvaluator.cs index 3aa47d1b..2391bd56 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Engines/RoslynVirtualTagEvaluator.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Engines/RoslynVirtualTagEvaluator.cs @@ -18,6 +18,19 @@ namespace ZB.MOM.WW.OtOpcUa.Host.Engines; /// fan-out between actors is owned by DependencyMuxActor, not by the eval engine. /// Cycle detection + cascade ordering live in ; this adapter /// stays single-tag scoped to keep 's message loop simple. +/// +/// +/// 02/S12 — the apply-boundary (driven by the host actor per +/// deploy generation) disposes cached evaluators while unchanged children may still be evaluating. +/// A clear landing between this adapter's GetOrCompile and the run's disposed guard would +/// otherwise surface as a spurious Failure. The compiled path is therefore retry-safe: an +/// from an in-flight evaluation re-fetches (recompiling the +/// same source into the current cache generation) and retries once. A second disposal mid-retry +/// (two applies inside one evaluation — not a realistic cadence) falls through to the failure path. +/// Residual (accepted, bounded to one generation): a stopping child can recompile its stale source +/// into the fresh cache for one generation; P7's expression-set gate means no-op redeploys no +/// longer open this window at all. +/// /// public sealed class RoslynVirtualTagEvaluator : IVirtualTagEvaluator, IScriptCacheOwner, IDisposable { @@ -63,27 +76,6 @@ public sealed class RoslynVirtualTagEvaluator : IVirtualTagEvaluator, IScriptCac : VirtualTagEvalResult.Ok(null); } - ScriptEvaluator evaluator; - try - { - evaluator = _cache.GetOrCompile(expression); - } - catch (CompilationErrorException ex) - { - _logger.LogWarning(ex, "VirtualTag {Id}: Roslyn compile failed", virtualTagId); - return VirtualTagEvalResult.Failure($"compile error: {ex.Message}"); - } - catch (ScriptSandboxViolationException ex) - { - _logger.LogWarning(ex, "VirtualTag {Id}: sandbox violation", virtualTagId); - return VirtualTagEvalResult.Failure($"sandbox violation: {ex.Message}"); - } - catch (Exception ex) - { - _logger.LogWarning(ex, "VirtualTag {Id}: compile threw", virtualTagId); - return VirtualTagEvalResult.Failure($"compile failure: {ex.Message}"); - } - var readCache = BuildReadCache(dependencies); // Per-evaluation script logger: bind both ScriptId and VirtualTagId from the virtual-tag id // (in the live path the script id equals the virtual-tag id) so the Script-log page can @@ -98,25 +90,71 @@ public sealed class RoslynVirtualTagEvaluator : IVirtualTagEvaluator, IScriptCac virtualTagId, path), logger: scriptLog); - try + // 02/S12: fetch + run inside a bounded loop so an ObjectDisposedException caused by a concurrent + // apply-boundary ClearCompiledScripts (which disposed the evaluator between GetOrCompile and the + // run's disposed guard) re-fetches and retries exactly once. The loop runs at most twice — the + // retry catch only matches attempt == 0. + for (var attempt = 0; ; attempt++) { - // Route through TimedScriptEvaluator (Task.Run + WaitAsync): a raw CancellationToken can't - // interrupt a CPU-bound/infinite-loop script (Roslyn scripts don't poll it), so the previous - // CTS-only path let one runaway script hang this actor forever. WaitAsync returns control when - // the wall-clock budget fires regardless of whether the inner task completes (the orphaned - // thread is the documented, accepted trade-off — TimedScriptEvaluator remarks). - var timed = new TimedScriptEvaluator(evaluator, _runTimeout); - var raw = timed.RunAsync(context).GetAwaiter().GetResult(); - return VirtualTagEvalResult.Ok(raw); - } - catch (ScriptTimeoutException) - { - return VirtualTagEvalResult.Failure($"script timed out after {_runTimeout.TotalSeconds:F1}s"); - } - catch (Exception ex) - { - _logger.LogWarning(ex, "VirtualTag {Id}: script execution threw", virtualTagId); - return VirtualTagEvalResult.Failure($"script threw: {ex.Message}"); + ScriptEvaluator evaluator; + try + { + evaluator = _cache.GetOrCompile(expression); + } + catch (CompilationErrorException ex) + { + _logger.LogWarning(ex, "VirtualTag {Id}: Roslyn compile failed", virtualTagId); + return VirtualTagEvalResult.Failure($"compile error: {ex.Message}"); + } + catch (ScriptSandboxViolationException ex) + { + _logger.LogWarning(ex, "VirtualTag {Id}: sandbox violation", virtualTagId); + return VirtualTagEvalResult.Failure($"sandbox violation: {ex.Message}"); + } + catch (ObjectDisposedException) + { + // The cache itself is being torn down (this evaluator is disposing) — a fetch-time + // disposal is a shutdown signal, not the apply-boundary race, so never retry. + return VirtualTagEvalResult.Failure("evaluator disposed"); + } + catch (Exception ex) + { + _logger.LogWarning(ex, "VirtualTag {Id}: compile threw", virtualTagId); + return VirtualTagEvalResult.Failure($"compile failure: {ex.Message}"); + } + + try + { + // Route through TimedScriptEvaluator (Task.Run + WaitAsync): a raw CancellationToken can't + // interrupt a CPU-bound/infinite-loop script (Roslyn scripts don't poll it), so the previous + // CTS-only path let one runaway script hang this actor forever. WaitAsync returns control when + // the wall-clock budget fires regardless of whether the inner task completes (the orphaned + // thread is the documented, accepted trade-off — TimedScriptEvaluator remarks). + var timed = new TimedScriptEvaluator(evaluator, _runTimeout); + var raw = timed.RunAsync(context).GetAwaiter().GetResult(); + return VirtualTagEvalResult.Ok(raw); + } + catch (ScriptTimeoutException) + { + return VirtualTagEvalResult.Failure($"script timed out after {_runTimeout.TotalSeconds:F1}s"); + } + catch (ObjectDisposedException) when (!_disposed && attempt == 0) + { + // S12: the apply-boundary ClearCompiledScripts disposed the evaluator between our + // GetOrCompile and the run's disposed-guard. Re-fetch — GetOrCompile recompiles the same + // source into the CURRENT cache generation — and retry once. A second disposal mid-retry + // (two applies inside one evaluation) falls through to the failure path below. + continue; + } + catch (ObjectDisposedException) + { + return VirtualTagEvalResult.Failure("evaluator disposed during evaluation"); + } + catch (Exception ex) + { + _logger.LogWarning(ex, "VirtualTag {Id}: script execution threw", virtualTagId); + return VirtualTagEvalResult.Failure($"script threw: {ex.Message}"); + } } }