From 4769e192e385667d6631d365cc1203ab6475f586 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Sun, 6 Sep 2026 10:57:09 -0400 Subject: [PATCH] fix(central-ui): Flow Debugger prediction no longer deadlocks the Blazor circuit MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit FlowDebugService.PredictMethodAsync resolved route targets with a sync-over-async GetAwaiter().GetResult() inside the predictor's synchronous resolver — on the circuit's single-threaded renderer sync context that blocked the dispatcher forever the moment a method whose script routes to a site script was chosen (every later click and JS-interop reply queued behind it; the page looked inert with a healthy websocket and no error). Misdiagnosed on 2026-09-06 as a login-disabled localStorage stall. Now a resolve-and-rewalk loop: each pass records the targets it could not answer, they are awaited between passes, and the walk converges once a pass asks for nothing new (bounded by MaxPredictionPasses). Regression test drives the service under a single-thread pump sync context (fails in 10 s on the old code, passes now) plus a nested Scripts.Call resolve-across-passes test. Claude-Session: https://claude.ai/code/session_01NcMG6xXdsJbmD2wYAgkK29 --- ...9-06-flow-debugger-followups.md.tasks.json | 103 ++++++++++++++-- .../Services/FlowDebugService.cs | 68 +++++++---- .../Services/FlowDebugServiceTests.cs | 111 ++++++++++++++++++ 3 files changed, 249 insertions(+), 33 deletions(-) diff --git a/docs/plans/2026-09-06-flow-debugger-followups.md.tasks.json b/docs/plans/2026-09-06-flow-debugger-followups.md.tasks.json index 8c655f2b..8700f528 100644 --- a/docs/plans/2026-09-06-flow-debugger-followups.md.tasks.json +++ b/docs/plans/2026-09-06-flow-debugger-followups.md.tasks.json @@ -1,17 +1,94 @@ { "planPath": "docs/plans/2026-09-06-flow-debugger-followups.md", "tasks": [ - {"id": 0, "subject": "Task 0: Diagnose the login-disabled interop stall (investigation)", "status": "pending"}, - {"id": 1, "subject": "Task 1: FlowStep.CorrelationId + CLI column", "status": "pending", "blockedBy": [0]}, - {"id": 2, "subject": "Task 2: Site Calls query — filter by TrackedOperationId", "status": "pending", "blockedBy": [0]}, - {"id": 3, "subject": "Task 3: Notification Outbox query — filter by NotificationId", "status": "pending", "blockedBy": [0]}, - {"id": 4, "subject": "Task 4: Site Calls page accepts ?correlationId=", "status": "pending", "blockedBy": [2]}, - {"id": 5, "subject": "Task 5: Notification report page accepts ?correlationId=", "status": "pending", "blockedBy": [3]}, - {"id": 6, "subject": "Task 6: FlowStepDetail — report deep links + Retry/Discard controls", "status": "pending", "blockedBy": [1]}, - {"id": 7, "subject": "Task 7: IFlowDebugService.RetryOutboundAsync / DiscardOutboundAsync", "status": "pending", "blockedBy": [1]}, - {"id": 8, "subject": "Task 8: Flow Debugger page — wire actions, confirm, scope check, re-poll", "status": "pending", "blockedBy": [6, 7]}, - {"id": 9, "subject": "Task 9: Playwright — browser Live run + localStorage restore (rig)", "status": "pending", "blockedBy": [8, 4, 5]}, - {"id": 10, "subject": "Task 10: Docs, register, memory", "status": "pending", "blockedBy": [8]} + { + "id": 0, + "subject": "Task 0: Diagnose the login-disabled interop stall (investigation)", + "status": "completed" + }, + { + "id": 1, + "subject": "Task 1: FlowStep.CorrelationId + CLI column", + "status": "pending", + "blockedBy": [ + 0 + ] + }, + { + "id": 2, + "subject": "Task 2: Site Calls query \u2014 filter by TrackedOperationId", + "status": "pending", + "blockedBy": [ + 0 + ] + }, + { + "id": 3, + "subject": "Task 3: Notification Outbox query \u2014 filter by NotificationId", + "status": "pending", + "blockedBy": [ + 0 + ] + }, + { + "id": 4, + "subject": "Task 4: Site Calls page accepts ?correlationId=", + "status": "pending", + "blockedBy": [ + 2 + ] + }, + { + "id": 5, + "subject": "Task 5: Notification report page accepts ?correlationId=", + "status": "pending", + "blockedBy": [ + 3 + ] + }, + { + "id": 6, + "subject": "Task 6: FlowStepDetail \u2014 report deep links + Retry/Discard controls", + "status": "pending", + "blockedBy": [ + 1 + ] + }, + { + "id": 7, + "subject": "Task 7: IFlowDebugService.RetryOutboundAsync / DiscardOutboundAsync", + "status": "pending", + "blockedBy": [ + 1 + ] + }, + { + "id": 8, + "subject": "Task 8: Flow Debugger page \u2014 wire actions, confirm, scope check, re-poll", + "status": "pending", + "blockedBy": [ + 6, + 7 + ] + }, + { + "id": 9, + "subject": "Task 9: Playwright \u2014 browser Live run + localStorage restore (rig)", + "status": "pending", + "blockedBy": [ + 8, + 4, + 5 + ] + }, + { + "id": 10, + "subject": "Task 10: Docs, register, memory", + "status": "pending", + "blockedBy": [ + 8 + ] + } ], - "lastUpdated": "2026-09-06T00:00:00Z" -} + "lastUpdated": "2026-09-06T15:12:00Z" +} \ No newline at end of file diff --git a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Services/FlowDebugService.cs b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Services/FlowDebugService.cs index b5de5d72..bb170697 100644 --- a/src/ZB.MOM.WW.ScadaBridge.CentralUI/Services/FlowDebugService.cs +++ b/src/ZB.MOM.WW.ScadaBridge.CentralUI/Services/FlowDebugService.cs @@ -61,6 +61,13 @@ public sealed class FlowDebugService : IFlowDebugService _logger = logger ?? throw new ArgumentNullException(nameof(logger)); } + /// + /// Upper bound on the resolve-and-rewalk passes of . Each pass can + /// only discover targets one nesting level deeper than the last, so the predictor's own depth cap + /// bounds the useful pass count; the constant is a safety net, not a tuning knob. + /// + internal const int MaxPredictionPasses = 8; + /// public async Task PredictMethodAsync(int apiMethodId, CancellationToken cancellationToken = default) { @@ -68,32 +75,53 @@ public sealed class FlowDebugService : IFlowDebugService if (method == null) return PredictedFlow.Empty; - // The predictor is synchronous and takes a synchronous resolver, so the instance → - // template → script lookups are pre-fetched lazily through a small memo that blocks - // on the repository — acceptable for a handful of constant route targets, and the - // predictor never asks for anything else. - var memo = new Dictionary<(string, string), string?>(); - string? Resolve(string instanceCode, string scriptName) + // The predictor is synchronous and takes a synchronous resolver, and this method runs on the + // Blazor circuit's single-threaded renderer sync context — so the instance → template → script + // lookups must NEVER block on the repository from inside the resolver. The original + // `.GetAwaiter().GetResult()` there deadlocked the whole circuit (every later click and JS + // interop reply queued behind the blocked dispatcher forever) the moment a method whose script + // routes to a site script was chosen; found 2026-09-06 while chasing a "login-disabled interop + // stall" that was really this. Instead the walk is repeated: each pass records the route targets + // it asked for and could not answer, those are awaited between passes, and the walk converges + // once a pass asks for nothing new. Cheap — a pass is a Roslyn syntax walk over a short script. + var memo = new Dictionary<(string Instance, string Script), string?>(); + var script = method.Script ?? string.Empty; + for (var pass = 0; pass < MaxPredictionPasses; pass++) { - var key = (instanceCode, scriptName); - if (memo.TryGetValue(key, out var cached)) - return cached; - string? body; - try + var requested = new List<(string Instance, string Script)>(); + string? Resolve(string instanceCode, string scriptName) { - body = ResolveSiteScriptAsync(instanceCode, scriptName, cancellationToken).GetAwaiter().GetResult(); - } - catch (Exception ex) - { - _logger.LogDebug(ex, "Flow prediction could not resolve {Instance}.{Script}", instanceCode, scriptName); - body = null; + var key = (instanceCode, scriptName); + if (memo.TryGetValue(key, out var cached)) + return cached; + if (!requested.Contains(key)) + requested.Add(key); + return null; } - memo[key] = body; - return body; + var flow = FlowShapePredictor.PredictInboundMethod(method.Name, script, Resolve); + if (requested.Count == 0) + return flow; + + foreach (var key in requested) + { + string? body; + try + { + body = await ResolveSiteScriptAsync(key.Instance, key.Script, cancellationToken); + } + catch (Exception ex) when (ex is not OperationCanceledException) + { + _logger.LogDebug(ex, "Flow prediction could not resolve {Instance}.{Script}", key.Instance, key.Script); + body = null; + } + + memo[key] = body; + } } - return FlowShapePredictor.PredictInboundMethod(method.Name, method.Script ?? string.Empty, Resolve); + // Pathological: a walk still discovering new targets past the pass cap — predict with what is known. + return FlowShapePredictor.PredictInboundMethod(method.Name, script, (i, s) => memo.GetValueOrDefault((i, s))); } /// diff --git a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Services/FlowDebugServiceTests.cs b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Services/FlowDebugServiceTests.cs index 5cb04144..cae654f8 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Services/FlowDebugServiceTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.CentralUI.Tests/Services/FlowDebugServiceTests.cs @@ -15,6 +15,7 @@ using ZB.MOM.WW.ScadaBridge.Commons.Types.Audit; using ZB.MOM.WW.ScadaBridge.Commons.Types.Debugging; using ZB.MOM.WW.ScadaBridge.Commons.Types.Enums; using ZB.MOM.WW.ScadaBridge.ManagementService; +using ZB.MOM.WW.ScadaBridge.ScriptAnalysis; using ZB.MOM.WW.ScadaBridge.Security; namespace ZB.MOM.WW.ScadaBridge.CentralUI.Tests.Services; @@ -199,6 +200,116 @@ public sealed class FlowDebugServiceTests : IDisposable Assert.Equal("Sim01.Cmd", flow.Nodes[3].Title); } + /// + /// Regression for the 2026-09-06 circuit deadlock: + /// runs on Blazor Server's single-threaded renderer sync context, and the route-target resolver used + /// to block on the repository with GetAwaiter().GetResult() from inside the synchronous + /// predictor walk. A repository continuation posted back to that context could then never run — + /// the whole circuit hung (every later click / JS-interop reply queued behind the blocked dispatcher). + /// The pump below IS that context: one thread, posts queued, a blocked callback starves the queue. + /// + [Fact] + public void PredictMethod_OnASingleThreadedSyncContext_NeverBlocksTheDispatcher() + { + _inboundApi.GetApiMethodByIdAsync(4, Arg.Any()) + .Returns(new ApiMethod("Delmia", """return await Route.To("Sim01").Call("Process", new { });""") { Id = 4 }); + // The instance lookup completes only after a Yield — i.e. only when the sync context gets to + // run the continuation. Blocking on it from the context's own thread is the deadlock. + _templates.GetInstanceByUniqueNameAsync("Sim01", Arg.Any()) + .Returns(_ => YieldThen(new Instance("Sim01") { Id = 9, TemplateId = 10, SiteId = 1 })); + _templates.GetScriptsByTemplateIdAsync(10, Arg.Any()) + .Returns(Task.FromResult>( + [ + new TemplateScript("Process", """await Attributes.SetAsync("Cmd", 1);""") { Id = 1, TemplateId = 10 }, + ])); + _templates.GetTemplateByIdAsync(10, Arg.Any()) + .Returns(new Template("Base") { Id = 10, ParentTemplateId = null }); + var svc = CreateService(reply: null); + + var pump = new SingleThreadPump(); + PredictedFlow? flow = null; + Exception? failure = null; + var done = new ManualResetEventSlim(); + var thread = new Thread(() => + { + try { flow = pump.Run(() => svc.PredictMethodAsync(4)); } + catch (Exception ex) { failure = ex; } + finally { done.Set(); } + }) { IsBackground = true, Name = "flow-predict-pump" }; + thread.Start(); + + Assert.True(done.Wait(TimeSpan.FromSeconds(10)), + "PredictMethodAsync deadlocked the single-threaded sync context (sync-over-async in the route resolver)."); + Assert.Null(failure); + Assert.NotNull(flow); + Assert.Contains(flow!.Nodes, n => n.Kind == FlowStepKind.ScriptRun && n.Title == "Sim01.Process"); + Assert.Contains(flow.Nodes, n => n.Kind == FlowStepKind.AttributeWrite && n.Title == "Sim01.Cmd"); + } + + [Fact] + public async Task PredictMethod_NestedRouteTargets_ResolveAcrossPasses() + { + // Process calls a second script on the same instance via Scripts.Call — discovered only once + // Process itself has been resolved, so the service needs a second resolve-and-rewalk pass. + _inboundApi.GetApiMethodByIdAsync(4, Arg.Any()) + .Returns(new ApiMethod("Delmia", """return await Route.To("Sim01").Call("Process", new { });""") { Id = 4 }); + _templates.GetInstanceByUniqueNameAsync("Sim01", Arg.Any()) + .Returns(new Instance("Sim01") { Id = 9, TemplateId = 10, SiteId = 1 }); + _templates.GetScriptsByTemplateIdAsync(10, Arg.Any()) + .Returns(Task.FromResult>( + [ + new TemplateScript("Process", """await Scripts.Call("Inner", new { });""") { Id = 1, TemplateId = 10 }, + new TemplateScript("Inner", """await Attributes.SetAsync("Cmd", 1);""") { Id = 2, TemplateId = 10 }, + ])); + _templates.GetTemplateByIdAsync(10, Arg.Any()) + .Returns(new Template("Base") { Id = 10, ParentTemplateId = null }); + var svc = CreateService(reply: null); + + var flow = await svc.PredictMethodAsync(4); + + var kinds = flow.Nodes.Select(n => n.Kind).ToList(); + Assert.Contains(FlowStepKind.AttributeWrite, kinds); + Assert.Contains(flow.Nodes, n => n.Title == "Sim01.Inner"); + Assert.Contains(flow.Nodes, n => n.Title == "Sim01.Cmd"); + } + + private static async Task YieldThen(Instance instance) + { + await Task.Yield(); + return instance; + } + + /// A Blazor-Server-shaped sync context: a single thread draining a queue of posted callbacks. + private sealed class SingleThreadPump : SynchronizationContext + { + private readonly System.Collections.Concurrent.BlockingCollection<(SendOrPostCallback Callback, object? State)> _queue = new(); + + public override void Post(SendOrPostCallback d, object? state) => _queue.Add((d, state)); + + public override void Send(SendOrPostCallback d, object? state) => throw new NotSupportedException(); + + public T Run(Func> body) + { + var previous = Current; + SetSynchronizationContext(this); + try + { + var task = body(); + while (!task.IsCompleted) + { + var (callback, state) = _queue.Take(); + callback(state); + } + + return task.GetAwaiter().GetResult(); + } + finally + { + SetSynchronizationContext(previous); + } + } + } + [Fact] public async Task PredictMethod_UnknownMethod_IsEmpty() {