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..a7569acd6 --- /dev/null +++ b/src/modules/Elsa.Studio.Workflows.Tests/WorkflowInstanceDesignerDisconnectRefreshTests.cs @@ -0,0 +1,792 @@ +using System.Diagnostics; +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.Models; +using Elsa.Studio.Workflows.Shared.Components; +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") }; + yield return new object[] { new OperationCanceledException("The operation was canceled.") }; + } + + [Theory] + [MemberData(nameof(CircuitGoneExceptions))] + public async Task RefreshTickStopsPeriodicRefreshWhenCircuitIsGone(Exception circuitGoneException) + { + var activityExecutionService = new RecordingActivityExecutionService(circuitGoneException); + var cut = RenderDesigner(activityExecutionService); + SetLastActivityExecution(cut.Instance, "node-1"); + using var timer = new Timer(_ => { }, null, Timeout.Infinite, Timeout.Infinite); + SetRefreshTimer(cut.Instance, timer); + + try + { + 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)); + } + finally + { + await ((IAsyncDisposable)cut.Instance).DisposeAsync(); + } + } + + [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); + 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] + [MemberData(nameof(CircuitGoneExceptions))] + public async Task ElapsedTickStopsElapsedTimerWhenCircuitIsGone(Exception circuitGoneException) + { + var activityExecutionService = new RecordingActivityExecutionService(); + var cut = RenderDesigner(activityExecutionService); + cut.Instance.ThrowOnRender = circuitGoneException; + using var timer = new Timer(_ => { }, null, Timeout.Infinite, Timeout.Infinite); + SetElapsedTimer(cut.Instance, timer); + + 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); + } + + /// + /// 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"); + + 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; + + // 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(); + + // 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)); + } + + /// + /// 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() + { + 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(); + } + } + + /// + /// 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() + { + 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() + { + 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(); + } + } + + /// + /// 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", + 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 const string RefreshTimerFieldName = "_refreshTimer"; + private const string ElapsedTimerFieldName = "_elapsedTimer"; + + private static void SetRefreshTimer(WorkflowInstanceDesigner instance, Timer timer) => + SetTimer(instance, RefreshTimerFieldName, timer); + + private static Timer? GetRefreshTimer(WorkflowInstanceDesigner instance) => + GetTimer(instance, RefreshTimerFieldName); + + private static Task InvokeRefreshTimerTickAsync(WorkflowInstanceDesigner instance, string activityExecutionRecordId) => + instance.RefreshTimerTickAsync(activityExecutionRecordId); + + private static void SetElapsedTimer(WorkflowInstanceDesigner instance, Timer timer) => + SetTimer(instance, ElapsedTimerFieldName, timer); + + private static Timer? GetElapsedTimer(WorkflowInstanceDesigner 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 GetTimerField(string fieldName) => + typeof(WorkflowInstanceDesigner).GetField(fieldName, BindingFlags.Instance | BindingFlags.NonPublic)!; + + 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 + { + 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; + } + } + + /// + /// 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 + { + add { } + remove { } + } + + public event Func? ActivityExecutionLogUpdated + { + add { } + remove { } + } + + public event Func? WorkflowInstanceUpdated + { + add { } + remove { } + } + + 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 + /// 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; } + public int NotifyStateChangedCallCount { get; private set; } + + protected override Task OnAfterRenderAsync(bool firstRender) => Task.CompletedTask; + + protected override void BuildRenderTree(RenderTreeBuilder builder) + { + } + + internal override Task NotifyStateChangedAsync() + { + NotifyStateChangedCallCount++; + return ThrowOnRender != null ? Task.FromException(ThrowOnRender) : base.NotifyStateChangedAsync(); + } + } + + /// + /// 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, + IEnumerable? summariesToReturn = null, + ActivityExecutionRecord? recordToReturn = 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(summariesToReturn ?? []); + } + + public Task GetAsync(string id, CancellationToken cancellationToken = default) => + 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(); + + 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..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 @@ -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 volatile bool _disposed; private bool IsAlterationsEnabled { get; set; } /// The workflow instance. @@ -82,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; @@ -204,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(); } } @@ -255,19 +302,99 @@ 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(_ => InvokeAsync(StateHasChanged), null, TimeSpan.Zero, TimeSpan.FromSeconds(1)); + if (_disposed) return; + if (_elapsedTimer != null) return; + + async void Callback(object? _) => await ElapsedTimerTickAsync(); + + // 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) + { + // 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(); + 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. + } } - private void StopElapsedTimer() + /// + /// 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() + { + 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 (_elapsedTimer != null) + if (_disposed) return false; + + try + { + await work(); + } + catch (Exception ex) when (IsCircuitGoneException(ex)) { - _elapsedTimer?.Dispose(); - _elapsedTimer = null; + // The circuit has disconnected (e.g. the browser tab hosting this workflow instance was + // 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; + } + + /// + /// 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() + { + var timer = Interlocked.Exchange(ref _elapsedTimer, null); + timer?.Dispose(); + } + + private Task StopElapsedTimerAsync() + { + StopElapsedTimer(); + return Task.CompletedTask; } private async Task HandleActivitySelectedAsync(JsonObject activity) @@ -342,30 +469,138 @@ 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) { - async void Callback(object? _) + if (_disposed) return; + + async void Callback(object? _) => await RefreshTimerTickAsync(activityExecutionRecordId); + + // 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)) { - await RefreshSelectedItemAsync(activityExecutionRecordId); + // 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; + } - if (LastActivityExecution == null || (LastActivityExecution.IsFused() && LastActivityExecution.Status != ActivityStatus.Running)) - await StopRefreshActivityStatePeriodically(); - else - _refreshTimer?.Change(TimeSpan.FromSeconds(1), Timeout.InfiniteTimeSpan); + 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. } + } - _refreshTimer = new(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. + /// Returns true when remains the published instance and should be + /// armed; false when it was detached again and must not be armed. + /// + private bool PublishRefreshTimer(Timer timer) + { + 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(); + return false; + } + + return true; } - private async Task StopRefreshActivityStatePeriodically() + /// + /// The body of the periodic refresh timer tick, extracted so tests can invoke it directly instead + /// of waiting for the real to fire. + /// + internal async Task RefreshTimerTickAsync(string activityExecutionRecordId) { - if (_refreshTimer != null) + var ticked = await RunTimerTickAsync(() => RefreshSelectedItemAsync(activityExecutionRecordId), StopRefreshTimerAsync); + + if (!ticked) return; + + if (_disposed) return; + + if (LastActivityExecution == null || (LastActivityExecution.IsFused() && LastActivityExecution.Status != ActivityStatus.Running)) { - await _refreshTimer.DisposeAsync(); - _refreshTimer = null; + // 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 + { + 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. + } + } + } + + /// + /// 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); + + if (timer is null) return; + + await timer.DisposeAsync(); } + /// + /// 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 +664,16 @@ private Task OnEditClicked() async ValueTask IAsyncDisposable.DisposeAsync() { + _disposed = true; StopElapsedTimer(); - await DisposeObserverAsync(); - if (_refreshTimer != null) - await _refreshTimer.DisposeAsync(); + try + { + await DisposeObserverAsync(); + } + finally + { + await StopRefreshActivityStatePeriodically(); + } } }