From ea61f37e5593858eef1c56659cfba77bf56e7451 Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Mon, 7 Sep 2026 12:35:42 +0200 Subject: [PATCH 01/12] fix(workflows): stop instance designer refresh timers when the circuit disconnects WorkflowInstanceDesigner's periodic activity-state refresh and elapsed-time timers kept firing after the component was disposed, and any exception raised while refreshing (ObjectDisposedException from a torn-down scoped service, JSDisconnectedException from a lost circuit, or OperationCanceledException) escaped the timer callback unhandled, crashing the process. Track disposal with a flag both timer callbacks check before doing any work, and treat those three exception types as "the circuit is gone": stop the periodic refresh and return quietly instead of letting them propagate. Other exceptions still surface as before. DisposeAsync now stops and disposes both timers and the observer. Refs #743 Co-Authored-By: Claude Fable 5.1 --- ...wInstanceDesignerDisconnectRefreshTests.cs | 207 ++++++++++++++++++ .../WorkflowInstanceDesigner.razor.cs | 55 ++++- 2 files changed, 251 insertions(+), 11 deletions(-) create mode 100644 src/modules/Elsa.Studio.Workflows.Tests/WorkflowInstanceDesignerDisconnectRefreshTests.cs diff --git a/src/modules/Elsa.Studio.Workflows.Tests/WorkflowInstanceDesignerDisconnectRefreshTests.cs b/src/modules/Elsa.Studio.Workflows.Tests/WorkflowInstanceDesignerDisconnectRefreshTests.cs new file mode 100644 index 000000000..4eadbf0ef --- /dev/null +++ b/src/modules/Elsa.Studio.Workflows.Tests/WorkflowInstanceDesignerDisconnectRefreshTests.cs @@ -0,0 +1,207 @@ +using System.Reflection; +using System.Text.Json.Nodes; +using Bunit; +using Elsa.Api.Client.Resources.ActivityExecutions.Models; +using Elsa.Api.Client.Resources.Resilience.Models; +using Elsa.Api.Client.Resources.WorkflowInstances.Enums; +using Elsa.Api.Client.Resources.WorkflowInstances.Models; +using Elsa.Api.Client.Shared.Models; +using Elsa.Studio.Contracts; +using Elsa.Studio.DomInterop.Contracts; +using Elsa.Studio.Localization; +using Elsa.Studio.Workflows.Components.WorkflowInstanceViewer.Components; +using Elsa.Studio.Workflows.Contracts; +using Elsa.Studio.Workflows.Domain.Contracts; +using Elsa.Studio.Workflows.UI.Contracts; +using Microsoft.AspNetCore.Components.Rendering; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Localization; +using Microsoft.JSInterop; +using Xunit; + +namespace Elsa.Studio.Workflows.Tests; + +/// +/// Pins that 's periodic activity-state refresh timer stops +/// quietly instead of crashing the process when the Blazor circuit it belongs to disconnects +/// (see https://github.com/elsa-workflows/elsa-studio/issues/743). +/// +public sealed class WorkflowInstanceDesignerDisconnectRefreshTests : BunitContext, IAsyncLifetime +{ + public WorkflowInstanceDesignerDisconnectRefreshTests() + { + JSInterop.Mode = JSRuntimeMode.Loose; + Services.AddSingleton(new TestLocalizer()); + Services.AddSingleton(new ActivityRegistryStub()); + Services.AddSingleton(new RemoteFeatureProviderStub()); + Services.AddSingleton(DispatchProxy.Create()); + Services.AddSingleton(DispatchProxy.Create()); + Services.AddSingleton(DispatchProxy.Create()); + Services.AddSingleton(DispatchProxy.Create()); + Services.AddSingleton(DispatchProxy.Create()); + Services.AddSingleton(DispatchProxy.Create()); + } + + Task IAsyncLifetime.InitializeAsync() => Task.CompletedTask; + async Task IAsyncLifetime.DisposeAsync() => await base.DisposeAsync(); + + [Fact] + public async Task RefreshTickAfterDisposalDoesNotThrowOrCallActivityExecutionService() + { + var activityExecutionService = new RecordingActivityExecutionService(); + var cut = RenderDesigner(activityExecutionService); + SetLastActivityExecution(cut.Instance, "node-1"); + + await ((IAsyncDisposable)cut.Instance).DisposeAsync(); + + var exception = await Record.ExceptionAsync(() => InvokeRefreshTimerTickAsync(cut.Instance, "exec-1")); + + Assert.Null(exception); + Assert.Equal(0, activityExecutionService.ListSummariesCallCount); + } + + public static IEnumerable CircuitGoneExceptions() + { + yield return new object[] { new JSDisconnectedException("The circuit has disconnected.") }; + yield return new object[] { new ObjectDisposedException("ActivityExecutionService") }; + } + + [Theory] + [MemberData(nameof(CircuitGoneExceptions))] + public async Task RefreshTickStopsPeriodicRefreshWhenCircuitIsGone(Exception circuitGoneException) + { + var activityExecutionService = new RecordingActivityExecutionService(circuitGoneException); + var cut = RenderDesigner(activityExecutionService); + SetLastActivityExecution(cut.Instance, "node-1"); + SetRefreshTimer(cut.Instance, new Timer(_ => { }, null, Timeout.Infinite, Timeout.Infinite)); + + var exception = await Record.ExceptionAsync(() => InvokeRefreshTimerTickAsync(cut.Instance, "exec-1")); + + Assert.Null(exception); + Assert.Equal(1, activityExecutionService.ListSummariesCallCount); + + // The periodic refresh timer has been stopped and disposed in response to the circuit-gone + // exception, so the real Timer can no longer produce a subsequent tick. + Assert.Null(GetRefreshTimer(cut.Instance)); + } + + private IRenderedComponent RenderDesigner(IActivityExecutionService activityExecutionService) + { + Services.AddSingleton(activityExecutionService); + + var workflowInstance = new WorkflowInstance + { + Id = "instance-1", + DefinitionId = "definition-1", + Status = WorkflowStatus.Finished + }; + + return Render(parameters => parameters + .Add(x => x.WorkflowInstance, workflowInstance)); + } + + private static void SetLastActivityExecution(WorkflowInstanceDesigner instance, string activityNodeId) + { + var property = typeof(WorkflowInstanceDesigner).GetProperty("LastActivityExecution", BindingFlags.Instance | BindingFlags.NonPublic)!; + property.SetValue(instance, new ActivityExecutionRecord + { + Id = "exec-1", + WorkflowInstanceId = "instance-1", + ActivityId = "activity-1", + ActivityNodeId = activityNodeId, + ActivityType = "Test", + Status = ActivityStatus.Running + }); + } + + private static void SetRefreshTimer(WorkflowInstanceDesigner instance, Timer timer) => + GetRefreshTimerField().SetValue(instance, timer); + + private static Timer? GetRefreshTimer(WorkflowInstanceDesigner instance) => + (Timer?)GetRefreshTimerField().GetValue(instance); + + private static FieldInfo GetRefreshTimerField() => + typeof(WorkflowInstanceDesigner).GetField("_refreshTimer", BindingFlags.Instance | BindingFlags.NonPublic)!; + + private static Task InvokeRefreshTimerTickAsync(WorkflowInstanceDesigner instance, string activityExecutionRecordId) + { + var method = typeof(WorkflowInstanceDesigner).GetMethod("RefreshTimerTickAsync", BindingFlags.Instance | BindingFlags.NonPublic)!; + return (Task)method.Invoke(instance, [activityExecutionRecordId])!; + } + + private sealed class TestWorkflowInstanceDesigner : WorkflowInstanceDesigner + { + protected override Task OnAfterRenderAsync(bool firstRender) => Task.CompletedTask; + + protected override void BuildRenderTree(RenderTreeBuilder builder) + { + } + } + + /// + /// An that counts calls to + /// and, when constructed with an exception, throws it from that call to simulate a circuit + /// disconnecting mid-refresh. + /// + private sealed class RecordingActivityExecutionService(Exception? exceptionToThrow = null) : IActivityExecutionService + { + public int ListSummariesCallCount { get; private set; } + + public Task GetReportAsync(string workflowInstanceId, JsonObject containerActivity, CancellationToken cancellationToken = default) => + throw new NotSupportedException(); + + public Task> ListAsync(string workflowInstanceId, string activityNodeId, CancellationToken cancellationToken = default) => + throw new NotSupportedException(); + + public Task> ListSummariesAsync(string workflowInstanceId, string activityNodeId, CancellationToken cancellationToken = default) + { + ListSummariesCallCount++; + + if (exceptionToThrow != null) + throw exceptionToThrow; + + return Task.FromResult>([]); + } + + public Task GetAsync(string id, CancellationToken cancellationToken = default) => + throw new NotSupportedException(); + + public Task GetCallStackAsync(string activityExecutionId, bool? includeCrossWorkflowChain = null, int? skip = null, int? take = null, CancellationToken cancellationToken = default) => + throw new NotSupportedException(); + + public Task> GetRetriesAsync(string activityInstanceId, int? skip = null, int? take = null, CancellationToken cancellationToken = default) => + throw new NotSupportedException(); + } + + private sealed class ActivityRegistryStub : IActivityRegistry + { + public Task RefreshAsync(CancellationToken cancellationToken = default) => throw new NotSupportedException(); + public Task EnsureLoadedAsync(CancellationToken cancellationToken = default) => Task.CompletedTask; + public IEnumerable List() => throw new NotSupportedException(); + public Elsa.Api.Client.Resources.ActivityDescriptors.Models.ActivityDescriptor? Find(string activityType, int? version = null) => throw new NotSupportedException(); + public IEnumerable FindAll(string activityType) => throw new NotSupportedException(); + public void MarkStale() => throw new NotSupportedException(); + } + + private sealed class RemoteFeatureProviderStub : IRemoteFeatureProvider + { + public Task IsEnabledAsync(string featureName, CancellationToken cancellationToken = default) => Task.FromResult(false); + public Task> ListAsync(CancellationToken cancellationToken = default) => throw new NotSupportedException(); + } + + private sealed class TestLocalizer : ILocalizer + { + public LocalizedString this[string? key] => new(key ?? string.Empty, key ?? string.Empty); + public LocalizedString this[string? key, params object[] arguments] => new(key ?? string.Empty, string.Format(key ?? string.Empty, arguments)); + } + + /// + /// A that throws for every call, used for services this component + /// depends on but that these tests never exercise. + /// + private class ThrowingProxy : DispatchProxy + { + protected override object? Invoke(MethodInfo? targetMethod, object?[]? args) => + throw new InvalidOperationException($"Unexpected call to {targetMethod!.DeclaringType!.Name}.{targetMethod.Name}."); + } +} diff --git a/src/modules/Elsa.Studio.Workflows/Components/WorkflowInstanceViewer/Components/WorkflowInstanceDesigner.razor.cs b/src/modules/Elsa.Studio.Workflows/Components/WorkflowInstanceViewer/Components/WorkflowInstanceDesigner.razor.cs index 9d034b878..24f345e9f 100644 --- a/src/modules/Elsa.Studio.Workflows/Components/WorkflowInstanceViewer/Components/WorkflowInstanceDesigner.razor.cs +++ b/src/modules/Elsa.Studio.Workflows/Components/WorkflowInstanceViewer/Components/WorkflowInstanceDesigner.razor.cs @@ -24,6 +24,7 @@ using Elsa.Studio.Workflows.UI.Contracts; using Elsa.Studio.Workflows.UI.Models; using Microsoft.AspNetCore.Components; +using Microsoft.JSInterop; using MudBlazor; using Radzen; using Radzen.Blazor; @@ -44,6 +45,7 @@ public partial class WorkflowInstanceDesigner : IAsyncDisposable private readonly Dictionary> _activityExecutionRecordsLookup = new(); private readonly Dictionary _lastActivityExecutionRecordLookup = new(); private Timer? _elapsedTimer; + private bool _disposed; private bool IsAlterationsEnabled { get; set; } /// The workflow instance. @@ -258,7 +260,11 @@ private async Task OnActivityExecutionLogUpdated(ActivityExecutionLogUpdatedMess private void StartElapsedTimer() { if (_elapsedTimer == null) - _elapsedTimer = new(_ => InvokeAsync(StateHasChanged), null, TimeSpan.Zero, TimeSpan.FromSeconds(1)); + _elapsedTimer = new(_ => + { + if (_disposed) return; + _ = InvokeAsync(StateHasChanged); + }, null, TimeSpan.Zero, TimeSpan.FromSeconds(1)); } private void StopElapsedTimer() @@ -344,17 +350,38 @@ await InvokeAsync(() => private void RefreshActivityStatePeriodically(string activityExecutionRecordId) { - async void Callback(object? _) + async void Callback(object? _) => await RefreshTimerTickAsync(activityExecutionRecordId); + + _refreshTimer = new(Callback, null, TimeSpan.FromSeconds(1), Timeout.InfiniteTimeSpan); + } + + /// + /// The body of the periodic refresh timer tick, extracted so tests can invoke it directly instead + /// of waiting for the real to fire. + /// + private async Task RefreshTimerTickAsync(string activityExecutionRecordId) + { + if (_disposed) return; + + try { await RefreshSelectedItemAsync(activityExecutionRecordId); - - if (LastActivityExecution == null || (LastActivityExecution.IsFused() && LastActivityExecution.Status != ActivityStatus.Running)) - await StopRefreshActivityStatePeriodically(); - else - _refreshTimer?.Change(TimeSpan.FromSeconds(1), Timeout.InfiniteTimeSpan); + } + catch (Exception ex) when (IsCircuitGoneException(ex)) + { + // The circuit has disconnected (e.g. the browser tab hosting this workflow instance was + // closed) while the refresh was in flight. Stop refreshing instead of letting the + // exception escape the timer callback and crash the process. + await StopRefreshActivityStatePeriodically(); + return; } - _refreshTimer = new(Callback, null, TimeSpan.FromSeconds(1), Timeout.InfiniteTimeSpan); + if (_disposed) return; + + if (LastActivityExecution == null || (LastActivityExecution.IsFused() && LastActivityExecution.Status != ActivityStatus.Running)) + await StopRefreshActivityStatePeriodically(); + else + _refreshTimer?.Change(TimeSpan.FromSeconds(1), Timeout.InfiniteTimeSpan); } private async Task StopRefreshActivityStatePeriodically() @@ -366,6 +393,13 @@ private async Task StopRefreshActivityStatePeriodically() } } + /// + /// Determines whether the given exception signals that the Blazor circuit is gone (disconnected + /// or already disposed), in which case timer callbacks should stop quietly instead of throwing. + /// + private static bool IsCircuitGoneException(Exception ex) => + ex is ObjectDisposedException or JSDisconnectedException or OperationCanceledException; + private static ActivityStats Map(ActivityExecutionStats source) { return new() @@ -429,10 +463,9 @@ private Task OnEditClicked() async ValueTask IAsyncDisposable.DisposeAsync() { + _disposed = true; StopElapsedTimer(); await DisposeObserverAsync(); - - if (_refreshTimer != null) - await _refreshTimer.DisposeAsync(); + await StopRefreshActivityStatePeriodically(); } } From e8509ce779cd69eba5ec0eeb97926c38387d144f Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Mon, 7 Sep 2026 12:52:43 +0200 Subject: [PATCH 02/12] fix(workflows): guard the elapsed timer against circuit disconnects and expose internal timer seams Co-Authored-By: Claude Fable 5.1 --- ...wInstanceDesignerDisconnectRefreshTests.cs | 61 +++++++++++++++++-- .../WorkflowInstanceDesigner.razor.cs | 36 +++++++++-- 2 files changed, 86 insertions(+), 11 deletions(-) diff --git a/src/modules/Elsa.Studio.Workflows.Tests/WorkflowInstanceDesignerDisconnectRefreshTests.cs b/src/modules/Elsa.Studio.Workflows.Tests/WorkflowInstanceDesignerDisconnectRefreshTests.cs index 4eadbf0ef..71e246746 100644 --- a/src/modules/Elsa.Studio.Workflows.Tests/WorkflowInstanceDesignerDisconnectRefreshTests.cs +++ b/src/modules/Elsa.Studio.Workflows.Tests/WorkflowInstanceDesignerDisconnectRefreshTests.cs @@ -64,6 +64,7 @@ public static IEnumerable CircuitGoneExceptions() { yield return new object[] { new JSDisconnectedException("The circuit has disconnected.") }; yield return new object[] { new ObjectDisposedException("ActivityExecutionService") }; + yield return new object[] { new OperationCanceledException("The operation was canceled.") }; } [Theory] @@ -85,6 +86,37 @@ public async Task RefreshTickStopsPeriodicRefreshWhenCircuitIsGone(Exception cir Assert.Null(GetRefreshTimer(cut.Instance)); } + [Fact] + public async Task ElapsedTickAfterDisposalDoesNothing() + { + var activityExecutionService = new RecordingActivityExecutionService(); + var cut = RenderDesigner(activityExecutionService); + + await ((IAsyncDisposable)cut.Instance).DisposeAsync(); + + var exception = await Record.ExceptionAsync(() => cut.Instance.ElapsedTimerTickAsync()); + + Assert.Null(exception); + } + + [Theory] + [MemberData(nameof(CircuitGoneExceptions))] + public async Task ElapsedTickStopsElapsedTimerWhenCircuitIsGone(Exception circuitGoneException) + { + var activityExecutionService = new RecordingActivityExecutionService(); + var cut = RenderDesigner(activityExecutionService); + cut.Instance.ThrowOnRender = circuitGoneException; + SetElapsedTimer(cut.Instance, new Timer(_ => { }, null, Timeout.Infinite, Timeout.Infinite)); + + var exception = await Record.ExceptionAsync(() => cut.Instance.ElapsedTimerTickAsync()); + + Assert.Null(exception); + + // The elapsed timer has been stopped and disposed in response to the circuit-gone exception, + // so the real Timer can no longer produce a subsequent tick. + Assert.Null(GetElapsedTimer(cut.Instance)); + } + private IRenderedComponent RenderDesigner(IActivityExecutionService activityExecutionService) { Services.AddSingleton(activityExecutionService); @@ -123,19 +155,38 @@ private static void SetRefreshTimer(WorkflowInstanceDesigner instance, Timer tim private static FieldInfo GetRefreshTimerField() => typeof(WorkflowInstanceDesigner).GetField("_refreshTimer", BindingFlags.Instance | BindingFlags.NonPublic)!; - private static Task InvokeRefreshTimerTickAsync(WorkflowInstanceDesigner instance, string activityExecutionRecordId) - { - var method = typeof(WorkflowInstanceDesigner).GetMethod("RefreshTimerTickAsync", BindingFlags.Instance | BindingFlags.NonPublic)!; - return (Task)method.Invoke(instance, [activityExecutionRecordId])!; - } + private static Task InvokeRefreshTimerTickAsync(WorkflowInstanceDesigner instance, string activityExecutionRecordId) => + instance.RefreshTimerTickAsync(activityExecutionRecordId); + private static void SetElapsedTimer(WorkflowInstanceDesigner instance, Timer timer) => + GetElapsedTimerField().SetValue(instance, timer); + + private static Timer? GetElapsedTimer(WorkflowInstanceDesigner instance) => + (Timer?)GetElapsedTimerField().GetValue(instance); + + private static FieldInfo GetElapsedTimerField() => + typeof(WorkflowInstanceDesigner).GetField("_elapsedTimer", BindingFlags.Instance | BindingFlags.NonPublic)!; + + /// + /// A whose state-changed notification can be made to throw + /// on demand. bUnit's test renderer does not propagate exceptions from the JS-interop-driven + /// render pipeline back through InvokeAsync(StateHasChanged) the way a real Blazor circuit + /// does, so the internal seam is + /// overridden here to simulate the circuit-gone exception that a real disconnect would surface + /// from that call. + /// private sealed class TestWorkflowInstanceDesigner : WorkflowInstanceDesigner { + public Exception? ThrowOnRender { get; set; } + protected override Task OnAfterRenderAsync(bool firstRender) => Task.CompletedTask; protected override void BuildRenderTree(RenderTreeBuilder builder) { } + + internal override Task NotifyStateChangedAsync() => + ThrowOnRender != null ? Task.FromException(ThrowOnRender) : base.NotifyStateChangedAsync(); } /// diff --git a/src/modules/Elsa.Studio.Workflows/Components/WorkflowInstanceViewer/Components/WorkflowInstanceDesigner.razor.cs b/src/modules/Elsa.Studio.Workflows/Components/WorkflowInstanceViewer/Components/WorkflowInstanceDesigner.razor.cs index 24f345e9f..79136ec02 100644 --- a/src/modules/Elsa.Studio.Workflows/Components/WorkflowInstanceViewer/Components/WorkflowInstanceDesigner.razor.cs +++ b/src/modules/Elsa.Studio.Workflows/Components/WorkflowInstanceViewer/Components/WorkflowInstanceDesigner.razor.cs @@ -260,13 +260,37 @@ private async Task OnActivityExecutionLogUpdated(ActivityExecutionLogUpdatedMess private void StartElapsedTimer() { if (_elapsedTimer == null) - _elapsedTimer = new(_ => - { - if (_disposed) return; - _ = InvokeAsync(StateHasChanged); - }, null, TimeSpan.Zero, TimeSpan.FromSeconds(1)); + _elapsedTimer = new(_ => _ = ElapsedTimerTickAsync(), null, TimeSpan.Zero, TimeSpan.FromSeconds(1)); + } + + /// + /// The body of the elapsed timer tick, extracted so tests can invoke it directly instead of + /// waiting for the real to fire. + /// + internal async Task ElapsedTimerTickAsync() + { + if (_disposed) return; + + try + { + await NotifyStateChangedAsync(); + } + catch (Exception ex) when (IsCircuitGoneException(ex)) + { + // The circuit has disconnected (e.g. the browser tab hosting this workflow instance was + // closed) while the render was in flight. Stop the elapsed timer instead of letting the + // exception escape the timer callback and crash the process. + StopElapsedTimer(); + } } + /// + /// Invokes on the renderer's dispatcher. Extracted as + /// a virtual seam so tests can simulate a circuit-gone exception surfacing from the render + /// pipeline without needing a real Blazor circuit. + /// + internal virtual Task NotifyStateChangedAsync() => InvokeAsync(StateHasChanged); + private void StopElapsedTimer() { if (_elapsedTimer != null) @@ -359,7 +383,7 @@ private void RefreshActivityStatePeriodically(string activityExecutionRecordId) /// The body of the periodic refresh timer tick, extracted so tests can invoke it directly instead /// of waiting for the real to fire. /// - private async Task RefreshTimerTickAsync(string activityExecutionRecordId) + internal async Task RefreshTimerTickAsync(string activityExecutionRecordId) { if (_disposed) return; From 32e07131ba557786fd4c7438cecb0de420bf696a Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Mon, 7 Sep 2026 12:58:18 +0200 Subject: [PATCH 03/12] refactor(workflows): share the timer tick guard and assert the disposed elapsed tick is a no-op Co-Authored-By: Claude Fable 5.1 --- ...wInstanceDesignerDisconnectRefreshTests.cs | 20 +++++++- .../WorkflowInstanceDesigner.razor.cs | 46 +++++++++++-------- 2 files changed, 46 insertions(+), 20 deletions(-) diff --git a/src/modules/Elsa.Studio.Workflows.Tests/WorkflowInstanceDesignerDisconnectRefreshTests.cs b/src/modules/Elsa.Studio.Workflows.Tests/WorkflowInstanceDesignerDisconnectRefreshTests.cs index 71e246746..8fdfa2b09 100644 --- a/src/modules/Elsa.Studio.Workflows.Tests/WorkflowInstanceDesignerDisconnectRefreshTests.cs +++ b/src/modules/Elsa.Studio.Workflows.Tests/WorkflowInstanceDesignerDisconnectRefreshTests.cs @@ -97,6 +97,18 @@ public async Task ElapsedTickAfterDisposalDoesNothing() var exception = await Record.ExceptionAsync(() => cut.Instance.ElapsedTimerTickAsync()); Assert.Null(exception); + Assert.Equal(0, cut.Instance.NotifyStateChangedCallCount); + } + + [Fact] + public async Task ElapsedTickBeforeDisposalNotifiesStateChanged() + { + var activityExecutionService = new RecordingActivityExecutionService(); + var cut = RenderDesigner(activityExecutionService); + + await cut.Instance.ElapsedTimerTickAsync(); + + Assert.Equal(1, cut.Instance.NotifyStateChangedCallCount); } [Theory] @@ -178,6 +190,7 @@ private static FieldInfo GetElapsedTimerField() => private sealed class TestWorkflowInstanceDesigner : WorkflowInstanceDesigner { public Exception? ThrowOnRender { get; set; } + public int NotifyStateChangedCallCount { get; private set; } protected override Task OnAfterRenderAsync(bool firstRender) => Task.CompletedTask; @@ -185,8 +198,11 @@ protected override void BuildRenderTree(RenderTreeBuilder builder) { } - internal override Task NotifyStateChangedAsync() => - ThrowOnRender != null ? Task.FromException(ThrowOnRender) : base.NotifyStateChangedAsync(); + internal override Task NotifyStateChangedAsync() + { + NotifyStateChangedCallCount++; + return ThrowOnRender != null ? Task.FromException(ThrowOnRender) : base.NotifyStateChangedAsync(); + } } /// diff --git a/src/modules/Elsa.Studio.Workflows/Components/WorkflowInstanceViewer/Components/WorkflowInstanceDesigner.razor.cs b/src/modules/Elsa.Studio.Workflows/Components/WorkflowInstanceViewer/Components/WorkflowInstanceDesigner.razor.cs index 79136ec02..22ce828c6 100644 --- a/src/modules/Elsa.Studio.Workflows/Components/WorkflowInstanceViewer/Components/WorkflowInstanceDesigner.razor.cs +++ b/src/modules/Elsa.Studio.Workflows/Components/WorkflowInstanceViewer/Components/WorkflowInstanceDesigner.razor.cs @@ -269,19 +269,34 @@ private void StartElapsedTimer() /// internal async Task ElapsedTimerTickAsync() { - if (_disposed) return; + await RunTimerTickAsync(NotifyStateChangedAsync, StopElapsedTimerAsync); + } + + /// + /// Runs the body of a periodic timer tick, guarding it against disposal and a disconnected + /// circuit. Returns false when the component was already disposed or when + /// threw a circuit-gone exception (in which case stopped the timer); + /// returns true when completed normally. Exceptions that do not + /// signal a gone circuit propagate to the caller. + /// + private async Task RunTimerTickAsync(Func work, Func stopTimer) + { + if (_disposed) return false; try { - await NotifyStateChangedAsync(); + await work(); } catch (Exception ex) when (IsCircuitGoneException(ex)) { // The circuit has disconnected (e.g. the browser tab hosting this workflow instance was - // closed) while the render was in flight. Stop the elapsed timer instead of letting the - // exception escape the timer callback and crash the process. - StopElapsedTimer(); + // closed) while the tick was in flight. Stop the timer instead of letting the exception + // escape the timer callback and crash the process. + await stopTimer(); + return false; } + + return true; } /// @@ -300,6 +315,12 @@ private void StopElapsedTimer() } } + private Task StopElapsedTimerAsync() + { + StopElapsedTimer(); + return Task.CompletedTask; + } + private async Task HandleActivitySelectedAsync(JsonObject activity) { await StopRefreshActivityStatePeriodically(); @@ -385,20 +406,9 @@ private void RefreshActivityStatePeriodically(string activityExecutionRecordId) /// internal async Task RefreshTimerTickAsync(string activityExecutionRecordId) { - if (_disposed) return; + var ticked = await RunTimerTickAsync(() => RefreshSelectedItemAsync(activityExecutionRecordId), StopRefreshActivityStatePeriodically); - try - { - await RefreshSelectedItemAsync(activityExecutionRecordId); - } - catch (Exception ex) when (IsCircuitGoneException(ex)) - { - // The circuit has disconnected (e.g. the browser tab hosting this workflow instance was - // closed) while the refresh was in flight. Stop refreshing instead of letting the - // exception escape the timer callback and crash the process. - await StopRefreshActivityStatePeriodically(); - return; - } + if (!ticked) return; if (_disposed) return; From 2054db3f59ad3b4bed52b08e2f5eabaf8d20f87f Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Mon, 7 Sep 2026 13:00:29 +0200 Subject: [PATCH 04/12] test(workflows): share the timer field reflection helpers Co-Authored-By: Claude Fable 5.1 --- ...wInstanceDesignerDisconnectRefreshTests.cs | 24 ++++++++++++------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/src/modules/Elsa.Studio.Workflows.Tests/WorkflowInstanceDesignerDisconnectRefreshTests.cs b/src/modules/Elsa.Studio.Workflows.Tests/WorkflowInstanceDesignerDisconnectRefreshTests.cs index 8fdfa2b09..191e25039 100644 --- a/src/modules/Elsa.Studio.Workflows.Tests/WorkflowInstanceDesignerDisconnectRefreshTests.cs +++ b/src/modules/Elsa.Studio.Workflows.Tests/WorkflowInstanceDesignerDisconnectRefreshTests.cs @@ -158,26 +158,32 @@ private static void SetLastActivityExecution(WorkflowInstanceDesigner instance, }); } + private const string RefreshTimerFieldName = "_refreshTimer"; + private const string ElapsedTimerFieldName = "_elapsedTimer"; + private static void SetRefreshTimer(WorkflowInstanceDesigner instance, Timer timer) => - GetRefreshTimerField().SetValue(instance, timer); + SetTimer(instance, RefreshTimerFieldName, timer); private static Timer? GetRefreshTimer(WorkflowInstanceDesigner instance) => - (Timer?)GetRefreshTimerField().GetValue(instance); - - private static FieldInfo GetRefreshTimerField() => - typeof(WorkflowInstanceDesigner).GetField("_refreshTimer", BindingFlags.Instance | BindingFlags.NonPublic)!; + GetTimer(instance, RefreshTimerFieldName); private static Task InvokeRefreshTimerTickAsync(WorkflowInstanceDesigner instance, string activityExecutionRecordId) => instance.RefreshTimerTickAsync(activityExecutionRecordId); private static void SetElapsedTimer(WorkflowInstanceDesigner instance, Timer timer) => - GetElapsedTimerField().SetValue(instance, timer); + SetTimer(instance, ElapsedTimerFieldName, timer); private static Timer? GetElapsedTimer(WorkflowInstanceDesigner instance) => - (Timer?)GetElapsedTimerField().GetValue(instance); + GetTimer(instance, ElapsedTimerFieldName); + + private static Timer? GetTimer(WorkflowInstanceDesigner instance, string fieldName) => + (Timer?)GetTimerField(fieldName).GetValue(instance); + + private static void SetTimer(WorkflowInstanceDesigner instance, string fieldName, Timer? value) => + GetTimerField(fieldName).SetValue(instance, value); - private static FieldInfo GetElapsedTimerField() => - typeof(WorkflowInstanceDesigner).GetField("_elapsedTimer", BindingFlags.Instance | BindingFlags.NonPublic)!; + private static FieldInfo GetTimerField(string fieldName) => + typeof(WorkflowInstanceDesigner).GetField(fieldName, BindingFlags.Instance | BindingFlags.NonPublic)!; /// /// A whose state-changed notification can be made to throw From bed9515e26789012818b7721b7906b8d8a1c1423 Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Mon, 7 Sep 2026 13:13:32 +0200 Subject: [PATCH 05/12] fix(workflows): detach timers atomically and tolerate a concurrently disposed refresh timer Co-Authored-By: Claude Fable 5.1 --- ...wInstanceDesignerDisconnectRefreshTests.cs | 101 +++++++++++++++--- .../WorkflowInstanceDesigner.razor.cs | 34 ++++-- 2 files changed, 110 insertions(+), 25 deletions(-) diff --git a/src/modules/Elsa.Studio.Workflows.Tests/WorkflowInstanceDesignerDisconnectRefreshTests.cs b/src/modules/Elsa.Studio.Workflows.Tests/WorkflowInstanceDesignerDisconnectRefreshTests.cs index 191e25039..db2bfe0f8 100644 --- a/src/modules/Elsa.Studio.Workflows.Tests/WorkflowInstanceDesignerDisconnectRefreshTests.cs +++ b/src/modules/Elsa.Studio.Workflows.Tests/WorkflowInstanceDesignerDisconnectRefreshTests.cs @@ -74,16 +74,24 @@ public async Task RefreshTickStopsPeriodicRefreshWhenCircuitIsGone(Exception cir var activityExecutionService = new RecordingActivityExecutionService(circuitGoneException); var cut = RenderDesigner(activityExecutionService); SetLastActivityExecution(cut.Instance, "node-1"); - SetRefreshTimer(cut.Instance, new Timer(_ => { }, null, Timeout.Infinite, Timeout.Infinite)); + using var timer = new Timer(_ => { }, null, Timeout.Infinite, Timeout.Infinite); + SetRefreshTimer(cut.Instance, timer); - var exception = await Record.ExceptionAsync(() => InvokeRefreshTimerTickAsync(cut.Instance, "exec-1")); + try + { + var exception = await Record.ExceptionAsync(() => InvokeRefreshTimerTickAsync(cut.Instance, "exec-1")); - Assert.Null(exception); - Assert.Equal(1, activityExecutionService.ListSummariesCallCount); + Assert.Null(exception); + Assert.Equal(1, activityExecutionService.ListSummariesCallCount); - // The periodic refresh timer has been stopped and disposed in response to the circuit-gone - // exception, so the real Timer can no longer produce a subsequent tick. - Assert.Null(GetRefreshTimer(cut.Instance)); + // The periodic refresh timer has been stopped and disposed in response to the circuit-gone + // exception, so the real Timer can no longer produce a subsequent tick. + Assert.Null(GetRefreshTimer(cut.Instance)); + } + finally + { + await ((IAsyncDisposable)cut.Instance).DisposeAsync(); + } } [Fact] @@ -118,14 +126,76 @@ public async Task ElapsedTickStopsElapsedTimerWhenCircuitIsGone(Exception circui var activityExecutionService = new RecordingActivityExecutionService(); var cut = RenderDesigner(activityExecutionService); cut.Instance.ThrowOnRender = circuitGoneException; - SetElapsedTimer(cut.Instance, new Timer(_ => { }, null, Timeout.Infinite, Timeout.Infinite)); + using var timer = new Timer(_ => { }, null, Timeout.Infinite, Timeout.Infinite); + SetElapsedTimer(cut.Instance, timer); - var exception = await Record.ExceptionAsync(() => cut.Instance.ElapsedTimerTickAsync()); + try + { + var exception = await Record.ExceptionAsync(() => cut.Instance.ElapsedTimerTickAsync()); + + Assert.Null(exception); + + // The elapsed timer has been stopped and disposed in response to the circuit-gone exception, + // so the real Timer can no longer produce a subsequent tick. + Assert.Null(GetElapsedTimer(cut.Instance)); + } + finally + { + await ((IAsyncDisposable)cut.Instance).DisposeAsync(); + } + } + + [Fact] + public async Task RefreshTimerTickRearmToleratesConcurrentlyDisposedTimer() + { + var runningRecord = new ActivityExecutionRecord + { + Id = "exec-1", + WorkflowInstanceId = "instance-1", + ActivityId = "activity-1", + ActivityNodeId = "node-1", + ActivityType = "Test", + Status = ActivityStatus.Running + }; + var summary = new ActivityExecutionRecordSummary + { + Id = "exec-1", + WorkflowInstanceId = "instance-1", + ActivityId = "activity-1", + ActivityNodeId = "node-1", + ActivityType = "Test", + Status = ActivityStatus.Running + }; + var activityExecutionService = new RecordingActivityExecutionService(summariesToReturn: [summary], recordToReturn: runningRecord); + var cut = RenderDesigner(activityExecutionService); + SetLastActivityExecution(cut.Instance, "node-1"); + + // Simulate the timer being disposed concurrently (e.g. by DisposeAsync racing this tick) right + // before the tick tries to rearm it. + var timer = new Timer(_ => { }, null, Timeout.Infinite, Timeout.Infinite); + SetRefreshTimer(cut.Instance, timer); + timer.Dispose(); + + var exception = await Record.ExceptionAsync(() => InvokeRefreshTimerTickAsync(cut.Instance, "exec-1")); Assert.Null(exception); + } - // The elapsed timer has been stopped and disposed in response to the circuit-gone exception, - // so the real Timer can no longer produce a subsequent tick. + [Fact] + public async Task ConcurrentDisposeCallsDetachTimersAtomicallyWithoutThrowing() + { + var activityExecutionService = new RecordingActivityExecutionService(); + var cut = RenderDesigner(activityExecutionService); + SetLastActivityExecution(cut.Instance, "node-1"); + SetRefreshTimer(cut.Instance, new Timer(_ => { }, null, Timeout.Infinite, Timeout.Infinite)); + SetElapsedTimer(cut.Instance, new Timer(_ => { }, null, Timeout.Infinite, Timeout.Infinite)); + + var disposable = (IAsyncDisposable)cut.Instance; + + var exception = await Record.ExceptionAsync(() => Task.WhenAll(disposable.DisposeAsync().AsTask(), disposable.DisposeAsync().AsTask())); + + Assert.Null(exception); + Assert.Null(GetRefreshTimer(cut.Instance)); Assert.Null(GetElapsedTimer(cut.Instance)); } @@ -216,7 +286,10 @@ internal override Task NotifyStateChangedAsync() /// and, when constructed with an exception, throws it from that call to simulate a circuit /// disconnecting mid-refresh. /// - private sealed class RecordingActivityExecutionService(Exception? exceptionToThrow = null) : IActivityExecutionService + private sealed class RecordingActivityExecutionService( + Exception? exceptionToThrow = null, + IEnumerable? summariesToReturn = null, + ActivityExecutionRecord? recordToReturn = null) : IActivityExecutionService { public int ListSummariesCallCount { get; private set; } @@ -233,11 +306,11 @@ public Task> ListSummariesAsync(stri if (exceptionToThrow != null) throw exceptionToThrow; - return Task.FromResult>([]); + return Task.FromResult(summariesToReturn ?? []); } public Task GetAsync(string id, CancellationToken cancellationToken = default) => - throw new NotSupportedException(); + recordToReturn != null ? Task.FromResult(recordToReturn) : throw new NotSupportedException(); public Task GetCallStackAsync(string activityExecutionId, bool? includeCrossWorkflowChain = null, int? skip = null, int? take = null, CancellationToken cancellationToken = default) => throw new NotSupportedException(); diff --git a/src/modules/Elsa.Studio.Workflows/Components/WorkflowInstanceViewer/Components/WorkflowInstanceDesigner.razor.cs b/src/modules/Elsa.Studio.Workflows/Components/WorkflowInstanceViewer/Components/WorkflowInstanceDesigner.razor.cs index 22ce828c6..2e5442e75 100644 --- a/src/modules/Elsa.Studio.Workflows/Components/WorkflowInstanceViewer/Components/WorkflowInstanceDesigner.razor.cs +++ b/src/modules/Elsa.Studio.Workflows/Components/WorkflowInstanceViewer/Components/WorkflowInstanceDesigner.razor.cs @@ -308,11 +308,8 @@ private async Task RunTimerTickAsync(Func work, Func stopTimer private void StopElapsedTimer() { - if (_elapsedTimer != null) - { - _elapsedTimer?.Dispose(); - _elapsedTimer = null; - } + var timer = Interlocked.Exchange(ref _elapsedTimer, null); + timer?.Dispose(); } private Task StopElapsedTimerAsync() @@ -413,18 +410,33 @@ internal async Task RefreshTimerTickAsync(string activityExecutionRecordId) if (_disposed) return; if (LastActivityExecution == null || (LastActivityExecution.IsFused() && LastActivityExecution.Status != ActivityStatus.Running)) + { await StopRefreshActivityStatePeriodically(); + } else - _refreshTimer?.Change(TimeSpan.FromSeconds(1), Timeout.InfiniteTimeSpan); + { + var timer = _refreshTimer; + + if (timer == null) return; + + try + { + timer.Change(TimeSpan.FromSeconds(1), Timeout.InfiniteTimeSpan); + } + catch (ObjectDisposedException) + { + // The timer was disposed concurrently (e.g. by DisposeAsync racing this tick); nothing to rearm. + } + } } private async Task StopRefreshActivityStatePeriodically() { - if (_refreshTimer != null) - { - await _refreshTimer.DisposeAsync(); - _refreshTimer = null; - } + var timer = Interlocked.Exchange(ref _refreshTimer, null); + + if (timer is null) return; + + await timer.DisposeAsync(); } /// From 1386988a75cb5358e1253e2dcf392b808d71e7de Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Mon, 7 Sep 2026 13:21:07 +0200 Subject: [PATCH 06/12] fix(workflows): make the disposal flag volatile and exercise overlapping disposals in the test Co-Authored-By: Claude Fable 5.1 --- ...wInstanceDesignerDisconnectRefreshTests.cs | 51 +++++++++++++++++-- .../WorkflowInstanceDesigner.razor.cs | 2 +- 2 files changed, 48 insertions(+), 5 deletions(-) diff --git a/src/modules/Elsa.Studio.Workflows.Tests/WorkflowInstanceDesignerDisconnectRefreshTests.cs b/src/modules/Elsa.Studio.Workflows.Tests/WorkflowInstanceDesignerDisconnectRefreshTests.cs index db2bfe0f8..f06cc6209 100644 --- a/src/modules/Elsa.Studio.Workflows.Tests/WorkflowInstanceDesignerDisconnectRefreshTests.cs +++ b/src/modules/Elsa.Studio.Workflows.Tests/WorkflowInstanceDesignerDisconnectRefreshTests.cs @@ -181,20 +181,63 @@ public async Task RefreshTimerTickRearmToleratesConcurrentlyDisposedTimer() Assert.Null(exception); } + /// + /// Pins that the refresh timer is detached from _refreshTimer atomically, before the + /// (potentially slow) call completes. To exercise that, the + /// refresh timer's callback is kept running (blocked on below) while + /// the first disposal is in flight, so a second, concurrent disposal genuinely overlaps with it + /// instead of running after the first has already finished. + /// [Fact] public async Task ConcurrentDisposeCallsDetachTimersAtomicallyWithoutThrowing() { + var timeout = TimeSpan.FromSeconds(5); var activityExecutionService = new RecordingActivityExecutionService(); var cut = RenderDesigner(activityExecutionService); SetLastActivityExecution(cut.Instance, "node-1"); - SetRefreshTimer(cut.Instance, new Timer(_ => { }, null, Timeout.Infinite, Timeout.Infinite)); - SetElapsedTimer(cut.Instance, new Timer(_ => { }, null, Timeout.Infinite, Timeout.Infinite)); + + using var started = new ManualResetEventSlim(false); + using var release = new ManualResetEventSlim(false); + using var refreshTimer = new Timer(_ => + { + started.Set(); + release.Wait(timeout); + }, null, Timeout.Infinite, Timeout.Infinite); + using var elapsedTimer = new Timer(_ => { }, null, Timeout.Infinite, Timeout.Infinite); + + SetRefreshTimer(cut.Instance, refreshTimer); + SetElapsedTimer(cut.Instance, elapsedTimer); + + // Fire the refresh timer's callback immediately and wait for it to actually start running. + refreshTimer.Change(TimeSpan.Zero, Timeout.InfiniteTimeSpan); + Assert.True(started.Wait(timeout), "The refresh timer callback did not start in time."); var disposable = (IAsyncDisposable)cut.Instance; - var exception = await Record.ExceptionAsync(() => Task.WhenAll(disposable.DisposeAsync().AsTask(), disposable.DisposeAsync().AsTask())); + // System.Threading.Timer.DisposeAsync only completes once any in-flight callback finishes, so + // this first disposal stays pending while the callback above is blocked on `release`. + var firstDisposeTask = disposable.DisposeAsync().AsTask(); - Assert.Null(exception); + // The atomic Interlocked.Exchange detach in StopRefreshActivityStatePeriodically happens + // before the timer is awaited, so the field is already cleared while the first disposal is + // still pending. Against the previous check/await/clear implementation, this assertion would + // still hold, but the second call below would then observe a non-null field and race to + // dispose/clear it itself instead of being a no-op. + Assert.Null(GetRefreshTimer(cut.Instance)); + + var secondDisposeException = await Record.ExceptionAsync(() => disposable.DisposeAsync().AsTask()); + + Assert.Null(secondDisposeException); + Assert.False(firstDisposeTask.IsCompleted, "The first disposal should still be pending on the blocked callback."); + + release.Set(); + + var completedTask = await Task.WhenAny(firstDisposeTask, Task.Delay(timeout)); + Assert.Same(firstDisposeTask, completedTask); + + var firstDisposeException = await Record.ExceptionAsync(() => firstDisposeTask); + + Assert.Null(firstDisposeException); Assert.Null(GetRefreshTimer(cut.Instance)); Assert.Null(GetElapsedTimer(cut.Instance)); } diff --git a/src/modules/Elsa.Studio.Workflows/Components/WorkflowInstanceViewer/Components/WorkflowInstanceDesigner.razor.cs b/src/modules/Elsa.Studio.Workflows/Components/WorkflowInstanceViewer/Components/WorkflowInstanceDesigner.razor.cs index 2e5442e75..533fdfe0c 100644 --- a/src/modules/Elsa.Studio.Workflows/Components/WorkflowInstanceViewer/Components/WorkflowInstanceDesigner.razor.cs +++ b/src/modules/Elsa.Studio.Workflows/Components/WorkflowInstanceViewer/Components/WorkflowInstanceDesigner.razor.cs @@ -45,7 +45,7 @@ public partial class WorkflowInstanceDesigner : IAsyncDisposable private readonly Dictionary> _activityExecutionRecordsLookup = new(); private readonly Dictionary _lastActivityExecutionRecordLookup = new(); private Timer? _elapsedTimer; - private bool _disposed; + private volatile bool _disposed; private bool IsAlterationsEnabled { get; set; } /// The workflow instance. From 474682831dec59931e94a3c0c20cb51b678bc16b Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Mon, 7 Sep 2026 13:28:23 +0200 Subject: [PATCH 07/12] fix(workflows): never install a designer timer after disposal Both StartElapsedTimer and RefreshActivityStatePeriodically could race with DisposeAsync: they checked _disposed, then created and published a Timer without re-checking, so a concurrent DisposeAsync between the check and the publish left a live timer rooting the disposed component. Both creators now publish the timer atomically via Interlocked and re-check _disposed afterward, detaching and disposing the timer if disposal happened in that window. Co-Authored-By: Claude Fable 5.1 --- ...wInstanceDesignerDisconnectRefreshTests.cs | 75 +++++++++++++++++++ .../WorkflowInstanceDesigner.razor.cs | 45 +++++++++-- 2 files changed, 115 insertions(+), 5 deletions(-) diff --git a/src/modules/Elsa.Studio.Workflows.Tests/WorkflowInstanceDesignerDisconnectRefreshTests.cs b/src/modules/Elsa.Studio.Workflows.Tests/WorkflowInstanceDesignerDisconnectRefreshTests.cs index f06cc6209..455a89033 100644 --- a/src/modules/Elsa.Studio.Workflows.Tests/WorkflowInstanceDesignerDisconnectRefreshTests.cs +++ b/src/modules/Elsa.Studio.Workflows.Tests/WorkflowInstanceDesignerDisconnectRefreshTests.cs @@ -242,6 +242,81 @@ public async Task ConcurrentDisposeCallsDetachTimersAtomicallyWithoutThrowing() Assert.Null(GetElapsedTimer(cut.Instance)); } + [Fact] + public async Task StartElapsedTimerAfterDisposalLeavesTimerFieldNull() + { + var activityExecutionService = new RecordingActivityExecutionService(); + var cut = RenderDesigner(activityExecutionService); + + await ((IAsyncDisposable)cut.Instance).DisposeAsync(); + + cut.Instance.StartElapsedTimer(); + + Assert.Null(GetElapsedTimer(cut.Instance)); + } + + [Fact] + public async Task StartElapsedTimerOnLiveComponentInstallsTimerOnce() + { + var activityExecutionService = new RecordingActivityExecutionService(); + var cut = RenderDesigner(activityExecutionService); + + try + { + cut.Instance.StartElapsedTimer(); + var firstTimer = GetElapsedTimer(cut.Instance); + Assert.NotNull(firstTimer); + + cut.Instance.StartElapsedTimer(); + var secondTimer = GetElapsedTimer(cut.Instance); + + Assert.Same(firstTimer, secondTimer); + } + finally + { + await ((IAsyncDisposable)cut.Instance).DisposeAsync(); + } + } + + [Fact] + public async Task RefreshActivityStatePeriodicallyAfterDisposalLeavesTimerFieldNull() + { + var activityExecutionService = new RecordingActivityExecutionService(); + var cut = RenderDesigner(activityExecutionService); + SetLastActivityExecution(cut.Instance, "node-1"); + + await ((IAsyncDisposable)cut.Instance).DisposeAsync(); + + cut.Instance.RefreshActivityStatePeriodically("exec-1"); + + Assert.Null(GetRefreshTimer(cut.Instance)); + } + + [Fact] + public async Task RefreshActivityStatePeriodicallyOnLiveComponentInstallsTimerOnce() + { + var activityExecutionService = new RecordingActivityExecutionService(); + var cut = RenderDesigner(activityExecutionService); + SetLastActivityExecution(cut.Instance, "node-1"); + + try + { + cut.Instance.RefreshActivityStatePeriodically("exec-1"); + var firstTimer = GetRefreshTimer(cut.Instance); + Assert.NotNull(firstTimer); + + cut.Instance.RefreshActivityStatePeriodically("exec-1"); + var secondTimer = GetRefreshTimer(cut.Instance); + + Assert.NotNull(secondTimer); + Assert.NotSame(firstTimer, secondTimer); + } + finally + { + await ((IAsyncDisposable)cut.Instance).DisposeAsync(); + } + } + private IRenderedComponent RenderDesigner(IActivityExecutionService activityExecutionService) { Services.AddSingleton(activityExecutionService); diff --git a/src/modules/Elsa.Studio.Workflows/Components/WorkflowInstanceViewer/Components/WorkflowInstanceDesigner.razor.cs b/src/modules/Elsa.Studio.Workflows/Components/WorkflowInstanceViewer/Components/WorkflowInstanceDesigner.razor.cs index 533fdfe0c..2cfdaa8c7 100644 --- a/src/modules/Elsa.Studio.Workflows/Components/WorkflowInstanceViewer/Components/WorkflowInstanceDesigner.razor.cs +++ b/src/modules/Elsa.Studio.Workflows/Components/WorkflowInstanceViewer/Components/WorkflowInstanceDesigner.razor.cs @@ -257,10 +257,30 @@ private async Task OnActivityExecutionLogUpdated(ActivityExecutionLogUpdatedMess } } - private void StartElapsedTimer() + /// + /// Starts the periodic elapsed-time timer, unless the component has already been disposed. Internal + /// so tests can invoke it directly to pin the disposal race it guards against. + /// + internal void StartElapsedTimer() { - if (_elapsedTimer == null) - _elapsedTimer = new(_ => _ = ElapsedTimerTickAsync(), null, TimeSpan.Zero, TimeSpan.FromSeconds(1)); + if (_disposed) return; + if (_elapsedTimer != null) return; + + var timer = new Timer(_ => _ = ElapsedTimerTickAsync(), null, TimeSpan.Zero, TimeSpan.FromSeconds(1)); + + if (Interlocked.CompareExchange(ref _elapsedTimer, timer, null) is not null) + { + // Another caller already installed a timer; discard the one we just created. + timer.Dispose(); + return; + } + + if (_disposed) + { + // DisposeAsync ran between the guard above and publishing the timer; detach and dispose it. + var disposedTimer = Interlocked.Exchange(ref _elapsedTimer, null); + disposedTimer?.Dispose(); + } } /// @@ -390,11 +410,26 @@ await InvokeAsync(() => }); } - private void RefreshActivityStatePeriodically(string activityExecutionRecordId) + /// + /// Starts the periodic activity-state refresh timer, unless the component has already been + /// disposed. Internal so tests can invoke it directly to pin the disposal race it guards against. + /// + internal void RefreshActivityStatePeriodically(string activityExecutionRecordId) { + if (_disposed) return; + async void Callback(object? _) => await RefreshTimerTickAsync(activityExecutionRecordId); - _refreshTimer = new(Callback, null, TimeSpan.FromSeconds(1), Timeout.InfiniteTimeSpan); + var timer = new Timer(Callback, null, TimeSpan.FromSeconds(1), Timeout.InfiniteTimeSpan); + var previousTimer = Interlocked.Exchange(ref _refreshTimer, timer); + previousTimer?.Dispose(); + + if (_disposed) + { + // DisposeAsync ran between the guard above and publishing the timer; detach and dispose it. + var disposedTimer = Interlocked.Exchange(ref _refreshTimer, null); + disposedTimer?.Dispose(); + } } /// From c99571a5ac4ca183dece394ec2ddb0e59e85b840 Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Mon, 7 Sep 2026 13:36:01 +0200 Subject: [PATCH 08/12] fix(workflows): surface elapsed tick failures and always stop the refresh timer on dispose Give the elapsed timer the same callback shape as the refresh timer so non-circuit-gone exceptions from ElapsedTimerTickAsync propagate instead of becoming unobserved task exceptions. Move refresh-timer cleanup into a finally block in DisposeAsync so it still runs when observer disposal faults. Route refresh-timer publication through a PublishRefreshTimer helper to make the timer's ownership/disposal path explicit for static analysis. Co-Authored-By: Claude Fable 5.1 --- ...wInstanceDesignerDisconnectRefreshTests.cs | 38 +++++++++++++++++++ .../WorkflowInstanceDesigner.razor.cs | 27 +++++++++++-- 2 files changed, 61 insertions(+), 4 deletions(-) diff --git a/src/modules/Elsa.Studio.Workflows.Tests/WorkflowInstanceDesignerDisconnectRefreshTests.cs b/src/modules/Elsa.Studio.Workflows.Tests/WorkflowInstanceDesignerDisconnectRefreshTests.cs index 455a89033..1818f660f 100644 --- a/src/modules/Elsa.Studio.Workflows.Tests/WorkflowInstanceDesignerDisconnectRefreshTests.cs +++ b/src/modules/Elsa.Studio.Workflows.Tests/WorkflowInstanceDesignerDisconnectRefreshTests.cs @@ -278,6 +278,25 @@ public async Task StartElapsedTimerOnLiveComponentInstallsTimerOnce() } } + [Fact] + public async Task DisposeAsyncStopsRefreshTimerEvenWhenObserverDisposalThrows() + { + var activityExecutionService = new RecordingActivityExecutionService(); + var cut = RenderDesigner(activityExecutionService); + SetLastActivityExecution(cut.Instance, "node-1"); + + var observerException = new InvalidOperationException("Observer disposal failed."); + SetWorkflowInstanceObserver(cut.Instance, new ThrowingWorkflowInstanceObserver(observerException)); + + using var refreshTimer = new Timer(_ => { }, null, Timeout.Infinite, Timeout.Infinite); + SetRefreshTimer(cut.Instance, refreshTimer); + + var exception = await Record.ExceptionAsync(() => ((IAsyncDisposable)cut.Instance).DisposeAsync().AsTask()); + + Assert.Same(observerException, exception); + Assert.Null(GetRefreshTimer(cut.Instance)); + } + [Fact] public async Task RefreshActivityStatePeriodicallyAfterDisposalLeavesTimerFieldNull() { @@ -373,6 +392,25 @@ private static void SetTimer(WorkflowInstanceDesigner instance, string fieldName private static FieldInfo GetTimerField(string fieldName) => typeof(WorkflowInstanceDesigner).GetField(fieldName, BindingFlags.Instance | BindingFlags.NonPublic)!; + private static void SetWorkflowInstanceObserver(WorkflowInstanceDesigner instance, IWorkflowInstanceObserver observer) + { + var property = typeof(WorkflowInstanceDesigner).GetProperty("WorkflowInstanceObserver", BindingFlags.Instance | BindingFlags.NonPublic)!; + property.SetValue(instance, observer); + } + + /// + /// An whose throws, used to pin + /// that the periodic refresh timer is still stopped when observer disposal faults. + /// + private sealed class ThrowingWorkflowInstanceObserver(Exception exceptionToThrow) : IWorkflowInstanceObserver + { + public event Func? WorkflowJournalUpdated; + public event Func? ActivityExecutionLogUpdated; + public event Func? WorkflowInstanceUpdated; + + public ValueTask DisposeAsync() => throw exceptionToThrow; + } + /// /// A whose state-changed notification can be made to throw /// on demand. bUnit's test renderer does not propagate exceptions from the JS-interop-driven diff --git a/src/modules/Elsa.Studio.Workflows/Components/WorkflowInstanceViewer/Components/WorkflowInstanceDesigner.razor.cs b/src/modules/Elsa.Studio.Workflows/Components/WorkflowInstanceViewer/Components/WorkflowInstanceDesigner.razor.cs index 2cfdaa8c7..7da0165ba 100644 --- a/src/modules/Elsa.Studio.Workflows/Components/WorkflowInstanceViewer/Components/WorkflowInstanceDesigner.razor.cs +++ b/src/modules/Elsa.Studio.Workflows/Components/WorkflowInstanceViewer/Components/WorkflowInstanceDesigner.razor.cs @@ -266,7 +266,9 @@ internal void StartElapsedTimer() if (_disposed) return; if (_elapsedTimer != null) return; - var timer = new Timer(_ => _ = ElapsedTimerTickAsync(), null, TimeSpan.Zero, TimeSpan.FromSeconds(1)); + async void Callback(object? _) => await ElapsedTimerTickAsync(); + + var timer = new Timer(Callback, null, TimeSpan.Zero, TimeSpan.FromSeconds(1)); if (Interlocked.CompareExchange(ref _elapsedTimer, timer, null) is not null) { @@ -420,7 +422,17 @@ internal void RefreshActivityStatePeriodically(string activityExecutionRecordId) async void Callback(object? _) => await RefreshTimerTickAsync(activityExecutionRecordId); - var timer = new Timer(Callback, null, TimeSpan.FromSeconds(1), Timeout.InfiniteTimeSpan); + // Ownership of the timer created here transfers to _refreshTimer via PublishRefreshTimer; it is + // disposed by the stop path (StopRefreshActivityStatePeriodically) or by DisposeAsync. + PublishRefreshTimer(new Timer(Callback, null, TimeSpan.FromSeconds(1), Timeout.InfiniteTimeSpan)); + } + + /// + /// Publishes a newly created refresh timer to , disposing any timer it + /// replaces, and detaches the published timer again if the component was disposed concurrently. + /// + private void PublishRefreshTimer(Timer timer) + { var previousTimer = Interlocked.Exchange(ref _refreshTimer, timer); previousTimer?.Dispose(); @@ -546,7 +558,14 @@ async ValueTask IAsyncDisposable.DisposeAsync() { _disposed = true; StopElapsedTimer(); - await DisposeObserverAsync(); - await StopRefreshActivityStatePeriodically(); + + try + { + await DisposeObserverAsync(); + } + finally + { + await StopRefreshActivityStatePeriodically(); + } } } From ba2777dcd965429e74a93e0a4c615ea5a9f56c24 Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Mon, 7 Sep 2026 13:37:04 +0200 Subject: [PATCH 09/12] test(workflows): satisfy observer events explicitly in the throwing stub Co-Authored-By: Claude Fable 5.1 --- ...wInstanceDesignerDisconnectRefreshTests.cs | 20 ++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/src/modules/Elsa.Studio.Workflows.Tests/WorkflowInstanceDesignerDisconnectRefreshTests.cs b/src/modules/Elsa.Studio.Workflows.Tests/WorkflowInstanceDesignerDisconnectRefreshTests.cs index 1818f660f..0184277e8 100644 --- a/src/modules/Elsa.Studio.Workflows.Tests/WorkflowInstanceDesignerDisconnectRefreshTests.cs +++ b/src/modules/Elsa.Studio.Workflows.Tests/WorkflowInstanceDesignerDisconnectRefreshTests.cs @@ -404,9 +404,23 @@ private static void SetWorkflowInstanceObserver(WorkflowInstanceDesigner instanc /// private sealed class ThrowingWorkflowInstanceObserver(Exception exceptionToThrow) : IWorkflowInstanceObserver { - public event Func? WorkflowJournalUpdated; - public event Func? ActivityExecutionLogUpdated; - public event Func? WorkflowInstanceUpdated; + public event Func? WorkflowJournalUpdated + { + add { } + remove { } + } + + public event Func? ActivityExecutionLogUpdated + { + add { } + remove { } + } + + public event Func? WorkflowInstanceUpdated + { + add { } + remove { } + } public ValueTask DisposeAsync() => throw exceptionToThrow; } From 745494ea98533414b4a8cab0ca73c09fda1c6e50 Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Mon, 7 Sep 2026 13:54:54 +0200 Subject: [PATCH 10/12] fix(workflows): dispose an observer created after the designer was disposed CreateObserverAsync awaited the observer factory before publishing the result, so a disposal that ran during that await left the new observer unregistered but still subscribed and unmanaged. Re-check the disposal flag after the factory call (and again after publishing the observer atomically) and dispose the observer instead of leaving it dangling. Co-Authored-By: Claude Fable 5.1 --- ...wInstanceDesignerDisconnectRefreshTests.cs | 153 +++++++++++++++++- .../WorkflowInstanceDesigner.razor.cs | 61 ++++++- 2 files changed, 202 insertions(+), 12 deletions(-) diff --git a/src/modules/Elsa.Studio.Workflows.Tests/WorkflowInstanceDesignerDisconnectRefreshTests.cs b/src/modules/Elsa.Studio.Workflows.Tests/WorkflowInstanceDesignerDisconnectRefreshTests.cs index 0184277e8..254b72e87 100644 --- a/src/modules/Elsa.Studio.Workflows.Tests/WorkflowInstanceDesignerDisconnectRefreshTests.cs +++ b/src/modules/Elsa.Studio.Workflows.Tests/WorkflowInstanceDesignerDisconnectRefreshTests.cs @@ -12,6 +12,8 @@ using Elsa.Studio.Workflows.Components.WorkflowInstanceViewer.Components; using Elsa.Studio.Workflows.Contracts; using Elsa.Studio.Workflows.Domain.Contracts; +using Elsa.Studio.Workflows.Models; +using Elsa.Studio.Workflows.Shared.Components; using Elsa.Studio.Workflows.UI.Contracts; using Microsoft.AspNetCore.Components.Rendering; using Microsoft.Extensions.DependencyInjection; @@ -336,10 +338,75 @@ public async Task RefreshActivityStatePeriodicallyOnLiveComponentInstallsTimerOn } } - private IRenderedComponent RenderDesigner(IActivityExecutionService activityExecutionService) + /// + /// Pins that a created by a factory call that was still in + /// flight when DisposeAsync ran is disposed immediately instead of being published and + /// subscribed on the torn-down component (see + /// https://github.com/elsa-workflows/elsa-studio/issues/743). + /// + [Fact] + public async Task CreateObserverAsyncDisposesObserverCreatedAfterDisposal() + { + var activityExecutionService = new RecordingActivityExecutionService(); + var factory = new GatedWorkflowInstanceObserverFactory(); + var cut = RenderDesigner(activityExecutionService, factory); + SetDesigner(cut.Instance, new JsonObject()); + + var createTask = cut.Instance.CreateObserverAsync(); + Assert.True(factory.CreateAsyncEntered.Wait(TimeSpan.FromSeconds(5)), "The factory was not called in time."); + + await ((IAsyncDisposable)cut.Instance).DisposeAsync(); + + var observer = new CountingWorkflowInstanceObserver(); + factory.Release(observer); + + await createTask; + + Assert.Null(GetWorkflowInstanceObserver(cut.Instance)); + Assert.Equal(1, observer.DisposeCallCount); + Assert.Equal(0, observer.SubscribeCount); + } + + /// + /// Keeps the live-circuit path exercised: when the component is not disposed, a created observer + /// is still published to and subscribed to. + /// + [Fact] + public async Task CreateObserverAsyncOnLiveComponentPublishesAndSubscribesObserver() + { + var activityExecutionService = new RecordingActivityExecutionService(); + var factory = new GatedWorkflowInstanceObserverFactory(); + var cut = RenderDesigner(activityExecutionService, factory); + SetDesigner(cut.Instance, new JsonObject()); + + var observer = new CountingWorkflowInstanceObserver(); + var createTask = cut.Instance.CreateObserverAsync(); + Assert.True(factory.CreateAsyncEntered.Wait(TimeSpan.FromSeconds(5)), "The factory was not called in time."); + factory.Release(observer); + + await createTask; + + try + { + Assert.Same(observer, GetWorkflowInstanceObserver(cut.Instance)); + Assert.Equal(1, observer.SubscribeCount); + Assert.Equal(0, observer.DisposeCallCount); + } + finally + { + await ((IAsyncDisposable)cut.Instance).DisposeAsync(); + } + } + + private IRenderedComponent RenderDesigner( + IActivityExecutionService activityExecutionService, + IWorkflowInstanceObserverFactory? observerFactory = null) { Services.AddSingleton(activityExecutionService); + if (observerFactory != null) + Services.AddSingleton(observerFactory); + var workflowInstance = new WorkflowInstance { Id = "instance-1", @@ -392,10 +459,88 @@ private static void SetTimer(WorkflowInstanceDesigner instance, string fieldName private static FieldInfo GetTimerField(string fieldName) => typeof(WorkflowInstanceDesigner).GetField(fieldName, BindingFlags.Instance | BindingFlags.NonPublic)!; - private static void SetWorkflowInstanceObserver(WorkflowInstanceDesigner instance, IWorkflowInstanceObserver observer) + private static void SetWorkflowInstanceObserver(WorkflowInstanceDesigner instance, IWorkflowInstanceObserver observer) => + GetWorkflowInstanceObserverProperty().SetValue(instance, observer); + + private static IWorkflowInstanceObserver? GetWorkflowInstanceObserver(WorkflowInstanceDesigner instance) => + (IWorkflowInstanceObserver?)GetWorkflowInstanceObserverProperty().GetValue(instance); + + private static PropertyInfo GetWorkflowInstanceObserverProperty() => + typeof(WorkflowInstanceDesigner).GetProperty("WorkflowInstanceObserver", BindingFlags.Instance | BindingFlags.NonPublic)!; + + /// + /// Attaches a bare (not rendered through bUnit, so its own + /// injected dependencies are never touched) to _designer, with its Activity parameter + /// set via reflection to avoid setting a component parameter outside of its render pipeline. + /// + private static void SetDesigner(WorkflowInstanceDesigner instance, JsonObject activity) + { + var designer = new DiagramDesignerWrapper(); + var activityProperty = typeof(DiagramDesignerWrapper).GetProperty(nameof(DiagramDesignerWrapper.Activity))!; + activityProperty.SetValue(designer, activity); + + var field = typeof(WorkflowInstanceDesigner).GetField("_designer", BindingFlags.Instance | BindingFlags.NonPublic)!; + field.SetValue(instance, designer); + } + + /// + /// An whose + /// blocks until is called, used to pin the window during which + /// WorkflowInstanceDesigner.CreateObserverAsync is awaiting the factory when disposal runs. + /// + private sealed class GatedWorkflowInstanceObserverFactory : IWorkflowInstanceObserverFactory + { + private readonly TaskCompletionSource _gate = new(TaskCreationOptions.RunContinuationsAsynchronously); + + /// Signaled once has been called. + public ManualResetEventSlim CreateAsyncEntered { get; } = new(false); + + public Task CreateAsync(string workflowInstanceId) => throw new NotSupportedException(); + + public Task CreateAsync(WorkflowInstanceObserverContext context) + { + CreateAsyncEntered.Set(); + return _gate.Task; + } + + /// Unblocks the pending call with the given observer. + public void Release(IWorkflowInstanceObserver observer) => _gate.SetResult(observer); + } + + /// + /// An that counts subscriptions and disposals, used to pin + /// that an observer created after disposal is disposed without being subscribed, while an observer + /// created on a live component is both subscribed and left undisposed. + /// + private sealed class CountingWorkflowInstanceObserver : IWorkflowInstanceObserver { - var property = typeof(WorkflowInstanceDesigner).GetProperty("WorkflowInstanceObserver", BindingFlags.Instance | BindingFlags.NonPublic)!; - property.SetValue(instance, observer); + public int DisposeCallCount { get; private set; } + public int SubscribeCount { get; private set; } + public int UnsubscribeCount { get; private set; } + + public event Func? WorkflowJournalUpdated + { + add { } + remove { } + } + + public event Func? ActivityExecutionLogUpdated + { + add => SubscribeCount++; + remove => UnsubscribeCount++; + } + + public event Func? WorkflowInstanceUpdated + { + add { } + remove { } + } + + public ValueTask DisposeAsync() + { + DisposeCallCount++; + return ValueTask.CompletedTask; + } } /// diff --git a/src/modules/Elsa.Studio.Workflows/Components/WorkflowInstanceViewer/Components/WorkflowInstanceDesigner.razor.cs b/src/modules/Elsa.Studio.Workflows/Components/WorkflowInstanceViewer/Components/WorkflowInstanceDesigner.razor.cs index 7da0165ba..e339fbdfe 100644 --- a/src/modules/Elsa.Studio.Workflows/Components/WorkflowInstanceViewer/Components/WorkflowInstanceDesigner.razor.cs +++ b/src/modules/Elsa.Studio.Workflows/Components/WorkflowInstanceViewer/Components/WorkflowInstanceDesigner.razor.cs @@ -84,7 +84,13 @@ public partial class WorkflowInstanceDesigner : IAsyncDisposable private JsonObject? SelectedActivity { get; set; } private ActivityDescriptor? ActivityDescriptor { get; set; } private JournalEntry? SelectedWorkflowExecutionLogRecord { get; set; } - private IWorkflowInstanceObserver? WorkflowInstanceObserver { get; set; } = null!; + private IWorkflowInstanceObserver? _workflowInstanceObserver; + + private IWorkflowInstanceObserver? WorkflowInstanceObserver + { + get => _workflowInstanceObserver; + set => _workflowInstanceObserver = value; + } private ICollection SelectedActivityExecutions { get; set; } = new List(); private ActivityExecutionRecord? LastActivityExecution { get; set; } private Timer? _refreshTimer; @@ -206,29 +212,68 @@ private async Task UpdateObserverAsync() } } - private async Task CreateObserverAsync() + /// + /// Creates and publishes a new , unless the component is or + /// becomes disposed while the factory call is in flight. Internal so tests can invoke it directly + /// to pin the disposal race it guards against. + /// + internal async Task CreateObserverAsync() { if (_workflowInstance == null || _designer == null) return; await DisposeObserverAsync(); + + if (_disposed) return; + var container = _designer.GetCurrentContainerActivityOrRoot(); var observerContext = new WorkflowInstanceObserverContext { WorkflowInstanceId = _workflowInstance.Id, ContainerActivity = container, }; - WorkflowInstanceObserver = await WorkflowInstanceObserverFactory.CreateAsync(observerContext); - WorkflowInstanceObserver.ActivityExecutionLogUpdated += OnActivityExecutionLogUpdated; + var observer = await WorkflowInstanceObserverFactory.CreateAsync(observerContext); + + if (_disposed) + { + // DisposeAsync ran while the factory call above was in flight; dispose the observer we just + // created instead of publishing and subscribing it on a torn-down component. + await observer.DisposeAsync(); + return; + } + + observer.ActivityExecutionLogUpdated += OnActivityExecutionLogUpdated; + + var previousObserver = Interlocked.Exchange(ref _workflowInstanceObserver, observer); + + if (previousObserver != null) + { + previousObserver.ActivityExecutionLogUpdated -= OnActivityExecutionLogUpdated; + await previousObserver.DisposeAsync(); + } + + if (_disposed) + { + // DisposeAsync ran between the guard above and publishing the observer; detach and dispose + // it, guarding against DisposeObserverAsync having already detached it. + var disposedObserver = Interlocked.Exchange(ref _workflowInstanceObserver, null); + + if (disposedObserver != null) + { + disposedObserver.ActivityExecutionLogUpdated -= OnActivityExecutionLogUpdated; + await disposedObserver.DisposeAsync(); + } + } } private async Task DisposeObserverAsync() { - if (WorkflowInstanceObserver != null!) + var observer = Interlocked.Exchange(ref _workflowInstanceObserver, null); + + if (observer != null) { - WorkflowInstanceObserver.ActivityExecutionLogUpdated -= OnActivityExecutionLogUpdated; - await WorkflowInstanceObserver.DisposeAsync(); - WorkflowInstanceObserver = null; + observer.ActivityExecutionLogUpdated -= OnActivityExecutionLogUpdated; + await observer.DisposeAsync(); } } From 89a98b3d2e23b7fddaf3db9e4907abbfe6265dc1 Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Mon, 7 Sep 2026 14:02:28 +0200 Subject: [PATCH 11/12] fix(workflows): publish designer timers before arming them Creates the elapsed and refresh timers disabled (Timeout.InfiniteTimeSpan), publishes them through the existing atomic publish path, re-checks for concurrent disposal, and only then arms them with Change(...) guarded by a try/catch(ObjectDisposedException). This closes the window where a timer's callback could fire and report a circuit-gone exception before CompareExchange/Exchange published it, causing StopElapsedTimer/ StopRefreshActivityStatePeriodically to see null while the timer went on to be published and rescheduled anyway. Co-Authored-By: Claude Fable 5.1 --- ...wInstanceDesignerDisconnectRefreshTests.cs | 66 +++++++++++++++++++ .../WorkflowInstanceDesigner.razor.cs | 41 ++++++++++-- 2 files changed, 102 insertions(+), 5 deletions(-) diff --git a/src/modules/Elsa.Studio.Workflows.Tests/WorkflowInstanceDesignerDisconnectRefreshTests.cs b/src/modules/Elsa.Studio.Workflows.Tests/WorkflowInstanceDesignerDisconnectRefreshTests.cs index 254b72e87..2b67c4561 100644 --- a/src/modules/Elsa.Studio.Workflows.Tests/WorkflowInstanceDesignerDisconnectRefreshTests.cs +++ b/src/modules/Elsa.Studio.Workflows.Tests/WorkflowInstanceDesignerDisconnectRefreshTests.cs @@ -280,6 +280,72 @@ public async Task StartElapsedTimerOnLiveComponentInstallsTimerOnce() } } + /// + /// Pins that arms the timer it publishes + /// (not just creates it disabled), by waiting for a real tick to reach + /// . + /// + [Fact] + public async Task StartElapsedTimerOnLiveComponentArmsTimerAndTicks() + { + var activityExecutionService = new RecordingActivityExecutionService(); + var cut = RenderDesigner(activityExecutionService); + + try + { + cut.Instance.StartElapsedTimer(); + + var ticked = await WaitUntilAsync(() => cut.Instance.NotifyStateChangedCallCount > 0, TimeSpan.FromSeconds(5)); + + Assert.True(ticked, "The elapsed timer did not tick within the bounded wait."); + } + finally + { + await ((IAsyncDisposable)cut.Instance).DisposeAsync(); + } + } + + /// + /// Pins that arms the timer + /// it publishes (not just creates it disabled), by waiting for a real tick to reach + /// . + /// + [Fact] + public async Task RefreshActivityStatePeriodicallyOnLiveComponentArmsTimerAndTicks() + { + var activityExecutionService = new RecordingActivityExecutionService(); + var cut = RenderDesigner(activityExecutionService); + SetLastActivityExecution(cut.Instance, "node-1"); + + try + { + cut.Instance.RefreshActivityStatePeriodically("exec-1"); + + var ticked = await WaitUntilAsync(() => activityExecutionService.ListSummariesCallCount > 0, TimeSpan.FromSeconds(5)); + + Assert.True(ticked, "The refresh timer did not tick within the bounded wait."); + } + finally + { + await ((IAsyncDisposable)cut.Instance).DisposeAsync(); + } + } + + private static async Task WaitUntilAsync(Func condition, TimeSpan timeout) + { + var deadline = DateTime.UtcNow + timeout; + + while (DateTime.UtcNow < deadline) + { + if (condition()) + return true; + + await Task.Delay(20); + } + + return condition(); + } + [Fact] public async Task DisposeAsyncStopsRefreshTimerEvenWhenObserverDisposalThrows() { diff --git a/src/modules/Elsa.Studio.Workflows/Components/WorkflowInstanceViewer/Components/WorkflowInstanceDesigner.razor.cs b/src/modules/Elsa.Studio.Workflows/Components/WorkflowInstanceViewer/Components/WorkflowInstanceDesigner.razor.cs index e339fbdfe..5e7c6d684 100644 --- a/src/modules/Elsa.Studio.Workflows/Components/WorkflowInstanceViewer/Components/WorkflowInstanceDesigner.razor.cs +++ b/src/modules/Elsa.Studio.Workflows/Components/WorkflowInstanceViewer/Components/WorkflowInstanceDesigner.razor.cs @@ -313,7 +313,9 @@ internal void StartElapsedTimer() async void Callback(object? _) => await ElapsedTimerTickAsync(); - var timer = new Timer(Callback, null, TimeSpan.Zero, TimeSpan.FromSeconds(1)); + // Create the timer disabled so its callback cannot fire before the timer is published to + // _elapsedTimer; it is armed only after publication succeeds. + var timer = new Timer(Callback, null, Timeout.InfiniteTimeSpan, Timeout.InfiniteTimeSpan); if (Interlocked.CompareExchange(ref _elapsedTimer, timer, null) is not null) { @@ -327,6 +329,16 @@ internal void StartElapsedTimer() // DisposeAsync ran between the guard above and publishing the timer; detach and dispose it. var disposedTimer = Interlocked.Exchange(ref _elapsedTimer, null); disposedTimer?.Dispose(); + return; + } + + try + { + timer.Change(TimeSpan.Zero, TimeSpan.FromSeconds(1)); + } + catch (ObjectDisposedException) + { + // The timer was disposed concurrently (e.g. by DisposeAsync racing this publish); nothing to arm. } } @@ -467,16 +479,32 @@ internal void RefreshActivityStatePeriodically(string activityExecutionRecordId) async void Callback(object? _) => await RefreshTimerTickAsync(activityExecutionRecordId); - // Ownership of the timer created here transfers to _refreshTimer via PublishRefreshTimer; it is - // disposed by the stop path (StopRefreshActivityStatePeriodically) or by DisposeAsync. - PublishRefreshTimer(new Timer(Callback, null, TimeSpan.FromSeconds(1), Timeout.InfiniteTimeSpan)); + // Create the timer disabled so its callback cannot fire before the timer is published to + // _refreshTimer; it is armed only after publication succeeds. Ownership of the timer created + // here transfers to _refreshTimer via PublishRefreshTimer; it is disposed by the stop path + // (StopRefreshActivityStatePeriodically) or by DisposeAsync. + var timer = new Timer(Callback, null, Timeout.InfiniteTimeSpan, Timeout.InfiniteTimeSpan); + + if (!PublishRefreshTimer(timer)) + return; + + try + { + timer.Change(TimeSpan.FromSeconds(1), Timeout.InfiniteTimeSpan); + } + catch (ObjectDisposedException) + { + // The timer was disposed concurrently (e.g. by DisposeAsync racing this publish); nothing to arm. + } } /// /// Publishes a newly created refresh timer to , disposing any timer it /// replaces, and detaches the published timer again if the component was disposed concurrently. + /// Returns true when remains the published instance and should be + /// armed; false when it was detached again and must not be armed. /// - private void PublishRefreshTimer(Timer timer) + private bool PublishRefreshTimer(Timer timer) { var previousTimer = Interlocked.Exchange(ref _refreshTimer, timer); previousTimer?.Dispose(); @@ -486,7 +514,10 @@ private void PublishRefreshTimer(Timer timer) // DisposeAsync ran between the guard above and publishing the timer; detach and dispose it. var disposedTimer = Interlocked.Exchange(ref _refreshTimer, null); disposedTimer?.Dispose(); + return false; } + + return true; } /// From fb90974c4ef22f0727e6d8602dcf07e56a1acafc Mon Sep 17 00:00:00 2001 From: Sipke Schoorstra Date: Mon, 7 Sep 2026 14:20:32 +0200 Subject: [PATCH 12/12] fix(workflows): stop the refresh timer without waiting when a tick stops it Timer.DisposeAsync() only completes once in-flight callbacks return, but RefreshTimerTickAsync's terminal-state branch and RunTimerTickAsync's stop delegate ran it from inside the refresh timer's own callback, which could deadlock the callback on its own completion. Split the stop into a non-waiting StopRefreshTimer() for those tick paths and keep the draining StopRefreshActivityStatePeriodically() for callers outside the callback (DisposeAsync, HandleActivitySelectedAsync). Also dispose the local timer on the false path of PublishRefreshTimer so static analysis sees every path disposing it. Co-Authored-By: Claude Fable 5.1 --- ...wInstanceDesignerDisconnectRefreshTests.cs | 58 +++++++++++++++++++ .../WorkflowInstanceDesigner.razor.cs | 36 +++++++++++- 2 files changed, 92 insertions(+), 2 deletions(-) diff --git a/src/modules/Elsa.Studio.Workflows.Tests/WorkflowInstanceDesignerDisconnectRefreshTests.cs b/src/modules/Elsa.Studio.Workflows.Tests/WorkflowInstanceDesignerDisconnectRefreshTests.cs index 2b67c4561..a7569acd6 100644 --- a/src/modules/Elsa.Studio.Workflows.Tests/WorkflowInstanceDesignerDisconnectRefreshTests.cs +++ b/src/modules/Elsa.Studio.Workflows.Tests/WorkflowInstanceDesignerDisconnectRefreshTests.cs @@ -1,3 +1,4 @@ +using System.Diagnostics; using System.Reflection; using System.Text.Json.Nodes; using Bunit; @@ -244,6 +245,63 @@ public async Task ConcurrentDisposeCallsDetachTimersAtomicallyWithoutThrowing() Assert.Null(GetElapsedTimer(cut.Instance)); } + /// + /// Pins that stopping the refresh timer from within its own tick (the path used by + /// 's terminal-state branch and by + /// 's stop delegate) does not wait for an in-flight callback to + /// return (see https://github.com/elsa-workflows/elsa-studio/issues/743). + /// only completes once any callback currently executing on the + /// timer has returned, regardless of which thread calls it, so awaiting it from the callback that + /// is itself executing would deadlock. This test keeps the timer's own callback blocked (simulating + /// it still being "in flight") and invokes the private, non-waiting stop method directly - + /// deliberately bypassing the render pipeline (InvokeAsync/StateHasChanged) so the + /// assertion is not confounded by ThreadPool contention between the blocked callback and the + /// renderer's dispatcher. Against an implementation that used the draining, DisposeAsync-based + /// stop from this path instead, the call below would block until the callback released. + /// + [Fact] + public void StoppingRefreshTimerFromTickPathDoesNotWaitForInFlightCallback() + { + var startTimeout = TimeSpan.FromSeconds(5); + var assertionBound = TimeSpan.FromMilliseconds(500); + var activityExecutionService = new RecordingActivityExecutionService(); + var cut = RenderDesigner(activityExecutionService); + + using var started = new ManualResetEventSlim(false); + using var release = new ManualResetEventSlim(false); + using var refreshTimer = new Timer(_ => + { + started.Set(); + + // Block for longer than the assertion bound below (but still bounded, so this thread is + // not tied up indefinitely if the assertion below fails), keeping the callback genuinely + // "in flight" for the whole window the assertion is checking. + release.Wait(TimeSpan.FromSeconds(10)); + }, null, Timeout.Infinite, Timeout.Infinite); + + SetRefreshTimer(cut.Instance, refreshTimer); + + // Fire the timer's own callback and wait for it to actually start running, so a genuine + // callback is in flight on the timer while the stop call below tries to stop it. + refreshTimer.Change(TimeSpan.Zero, Timeout.InfiniteTimeSpan); + Assert.True(started.Wait(startTimeout), "The refresh timer callback did not start in time."); + + try + { + var stopMethod = typeof(WorkflowInstanceDesigner).GetMethod("StopRefreshTimer", BindingFlags.Instance | BindingFlags.NonPublic)!; + var stopwatch = Stopwatch.StartNew(); + + stopMethod.Invoke(cut.Instance, null); + + Assert.True(stopwatch.Elapsed < assertionBound, $"Stopping the timer from the tick path took {stopwatch.Elapsed}, which suggests it waited for the blocked callback."); + Assert.Null(GetRefreshTimer(cut.Instance)); + } + finally + { + release.Set(); + } + } + [Fact] public async Task StartElapsedTimerAfterDisposalLeavesTimerFieldNull() { diff --git a/src/modules/Elsa.Studio.Workflows/Components/WorkflowInstanceViewer/Components/WorkflowInstanceDesigner.razor.cs b/src/modules/Elsa.Studio.Workflows/Components/WorkflowInstanceViewer/Components/WorkflowInstanceDesigner.razor.cs index 5e7c6d684..885f04101 100644 --- a/src/modules/Elsa.Studio.Workflows/Components/WorkflowInstanceViewer/Components/WorkflowInstanceDesigner.razor.cs +++ b/src/modules/Elsa.Studio.Workflows/Components/WorkflowInstanceViewer/Components/WorkflowInstanceDesigner.razor.cs @@ -486,7 +486,13 @@ internal void RefreshActivityStatePeriodically(string activityExecutionRecordId) var timer = new Timer(Callback, null, Timeout.InfiniteTimeSpan, Timeout.InfiniteTimeSpan); if (!PublishRefreshTimer(timer)) + { + // PublishRefreshTimer already disposes the timer it detaches on this path; dispose it here + // too so static analysis can see the local is disposed on every path (a second Timer.Dispose() + // call is a safe no-op). + timer.Dispose(); return; + } try { @@ -526,7 +532,7 @@ private bool PublishRefreshTimer(Timer timer) /// internal async Task RefreshTimerTickAsync(string activityExecutionRecordId) { - var ticked = await RunTimerTickAsync(() => RefreshSelectedItemAsync(activityExecutionRecordId), StopRefreshActivityStatePeriodically); + var ticked = await RunTimerTickAsync(() => RefreshSelectedItemAsync(activityExecutionRecordId), StopRefreshTimerAsync); if (!ticked) return; @@ -534,7 +540,9 @@ internal async Task RefreshTimerTickAsync(string activityExecutionRecordId) if (LastActivityExecution == null || (LastActivityExecution.IsFused() && LastActivityExecution.Status != ActivityStatus.Running)) { - await StopRefreshActivityStatePeriodically(); + // Called from the tick itself: use the non-waiting stop so this callback does not await + // its own completion (Timer.DisposeAsync waits for in-flight callbacks to return). + StopRefreshTimer(); } else { @@ -553,6 +561,30 @@ internal async Task RefreshTimerTickAsync(string activityExecutionRecordId) } } + /// + /// Stops the refresh timer without waiting for an in-flight callback to return. Use this from + /// within the timer's own callback (): + /// only completes once active callbacks return, so awaiting it from the callback that is currently + /// executing would deadlock the callback on its own completion. + /// + private void StopRefreshTimer() + { + var timer = Interlocked.Exchange(ref _refreshTimer, null); + timer?.Dispose(); + } + + private Task StopRefreshTimerAsync() + { + StopRefreshTimer(); + return Task.CompletedTask; + } + + /// + /// Stops the refresh timer and drains any in-flight callback before returning. Use this only from + /// callers that are not themselves executing on the timer callback (e.g. + /// or a non-timer caller), since waits for active callbacks to + /// finish. + /// private async Task StopRefreshActivityStatePeriodically() { var timer = Interlocked.Exchange(ref _refreshTimer, null);