diff --git a/src/Aspire.Cli/Backchannel/ExtensionRpcTarget.cs b/src/Aspire.Cli/Backchannel/ExtensionRpcTarget.cs index da4a65e2218..d1c0bb9e9b3 100644 --- a/src/Aspire.Cli/Backchannel/ExtensionRpcTarget.cs +++ b/src/Aspire.Cli/Backchannel/ExtensionRpcTarget.cs @@ -29,7 +29,10 @@ internal interface IExtensionRpcTarget Task GetCliCapabilitiesAsync(); } -internal class ExtensionRpcTarget(IConfiguration configuration, CliExecutionContext executionContext) : IExtensionRpcTarget +internal class ExtensionRpcTarget( + IConfiguration configuration, + CliExecutionContext executionContext, + ConsoleCancellationManager cancellationManager) : IExtensionRpcTarget { public Func? ValidationFunction { get; set; } @@ -45,7 +48,9 @@ public Task GetCliVersionAsync() public Task StopCliAsync() { - Environment.Exit(CliExitCodes.Success); + // The extension's stop request is cooperative. Route it through the same cancellation + // path as Ctrl+C so in-flight child processes release their workspace handles before exit. + cancellationManager.Cancel(CliExitCodes.Success); return Task.CompletedTask; } diff --git a/src/Aspire.Cli/Commands/RunCommand.cs b/src/Aspire.Cli/Commands/RunCommand.cs index 047ae35e83c..3dc6992dd2a 100644 --- a/src/Aspire.Cli/Commands/RunCommand.cs +++ b/src/Aspire.Cli/Commands/RunCommand.cs @@ -74,6 +74,7 @@ internal sealed class RunCommand : BaseCommand private readonly ProfilingTelemetry _profilingTelemetry; private readonly ProfileCaptureState _profileCaptureState; private readonly TimeProvider _timeProvider; + private readonly ConsoleCancellationManager _cancellationManager; private bool _isDetachMode; private const int MaxDisplayedAppHostStartupOutputLines = 80; // Match BackchannelLoggerProvider's 1,000-entry replay buffer. @@ -156,6 +157,7 @@ public RunCommand( _profilingTelemetry = profilingTelemetry; _profileCaptureState = profileCaptureState; _timeProvider = timeProvider; + _cancellationManager = services.CancellationManager; Options.Add(s_detachOption); Options.Add(s_noBuildOption); @@ -250,6 +252,32 @@ await extensionInteractionService.StartDebugSessionAsync( LauncherLivenessMonitor? launcherMonitor = null; Task? runTask = null; CancellationTokenSource? runCts = null; + CancellationTokenRegistration runCancellationRegistration = default; + var cancellationRequested = false; + TimeSpan? completedCancellationCleanupTimeout = null; + + async Task DrainCancelledRunAsync() + { + if ((cancellationRequested || cancellationToken.IsCancellationRequested) && + runCts is not null && + runTask is { IsCompleted: false }) + { + // BaseCommand owns the manager's process-wide shutdown deadline. + // Direct callers with an unrelated token still need a local bound. + var cleanupTimeout = _cancellationManager.IsCancellationRequested + ? Timeout.InfiniteTimeSpan + : s_appHostStartupCancellationTimeout; + // Do not repeat a completed bounded wait, but allow a later manager stop + // to upgrade it to the manager-owned drain. + if (completedCancellationCleanupTimeout == cleanupTimeout) + { + return; + } + + await CancelAppHostRunAsync(runCts, runTask, cleanupTimeout, CancellationToken.None).ConfigureAwait(false); + completedCancellationCleanupTimeout = cleanupTimeout; + } + } try { @@ -347,7 +375,8 @@ await extensionInteractionService.StartDebugSessionAsync( // Start the project run as a pending task - we'll handle UX while it runs var startupTimeout = TimeSpan.FromSeconds(timeoutSeconds); - runCts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); + runCts = new CancellationTokenSource(); + runCancellationRegistration = cancellationToken.Register(runCts.Cancel); // When this is a detached child, watch the foreground launcher during startup. If the launcher // is killed before the app is ready, cancel the run so the AppHost tree is torn down instead of leaking. @@ -439,7 +468,7 @@ await extensionInteractionService.StartDebugSessionAsync( catch (TimeoutException) { runActivity?.SetTag(TelemetryConstants.Tags.ErrorType, "startup_timeout"); - await CancelAppHostStartupAsync(runCts, runTask, cancellationToken).ConfigureAwait(false); + await CancelAppHostRunAsync(runCts, runTask, s_appHostStartupCancellationTimeout, cancellationToken).ConfigureAwait(false); return CreateStartupTimeoutResult(timeoutSeconds); } @@ -639,16 +668,6 @@ await ProcessResourceStatesAsync((resource, endpoint) => : CommandResult.FromExitCode(exitCode); } } - catch (OperationCanceledException ex) when (ex.CancellationToken == runCts.Token && cancellationToken.IsCancellationRequested) - { - runActivity?.SetTag(TelemetryConstants.Tags.ErrorType, "canceled"); - - // User Ctrl+C is the normal exit path for `aspire run`; surface as success. - // Internal failures `return X` directly from GuestAppHostProject.RunAsync rather - // than flowing through this catch, so we don't need to distinguish failure codes - // here. - return CommandResult.Cancelled(CliExitCodes.Success); - } finally { logCaptureCancellationSource.Cancel(); @@ -671,6 +690,7 @@ ex is ExtensionOperationCanceledException || (runCts is not null && ex.CancellationToken == runCts.Token && cancellationToken.IsCancellationRequested)) { runActivity?.SetTag(TelemetryConstants.Tags.ErrorType, "canceled"); + cancellationRequested = true; // User Ctrl+C is the normal exit path for `aspire run`; surface as success. // Internal failures `return X` directly from GuestAppHostProject.RunAsync rather @@ -718,29 +738,56 @@ ex is ExtensionOperationCanceledException || } finally { - if (IsDetachedStartChild() && runTask is { IsCompleted: false } detachedAppHostRun) + try { - // If the runTask is still running here, that is an abnormal exit. - // Cancel the run and wait for the AppHost to teardown so we don't leak child processes. try { - runCts?.Cancel(); - // CancellationToken.None is deliberate: root token is already cancelled. - await detachedAppHostRun.WaitAsync(s_detachedAppHostTeardownTimeout, _timeProvider, CancellationToken.None).ConfigureAwait(false); + // Keep the existing cancellation-first ordering and timeout budgets. + // The fenced call below handles cancellation arriving during later teardown. + await DrainCancelledRunAsync().ConfigureAwait(false); + + if (IsDetachedStartChild() && runTask is { IsCompleted: false } detachedAppHostRun) + { + // If the runTask is still running here, that is an abnormal exit. + // Cancel the run and wait for teardown so we don't leak child processes. + try + { + runCts?.Cancel(); + // Root cancellation must not interrupt this bounded detached-child drain. + await detachedAppHostRun.WaitAsync(s_detachedAppHostTeardownTimeout, _timeProvider, CancellationToken.None).ConfigureAwait(false); + } + catch (Exception ex) + { + _logger.LogDebug(ex, "Detached child timed out or failed while awaiting AppHost teardown during early exit."); + } + } } - catch (Exception ex) + finally { - _logger.LogDebug(ex, "Detached child timed out or failed while awaiting AppHost teardown during early exit."); + if (launcherMonitor is not null) + { + await launcherMonitor.DisposeAsync().ConfigureAwait(false); + } } } - - if (launcherMonitor is not null) + finally { - await launcherMonitor.DisposeAsync().ConfigureAwait(false); + // Close cancellation forwarding before deciding whether a drain is needed. + // DisposeAsync waits for an in-flight callback; cancellation after this fence + // cannot reach runCts and start child cleanup after we have decided to skip it. + await runCancellationRegistration.DisposeAsync().ConfigureAwait(false); + try + { + // Preserve the selected error result, including failures in final teardown, + // while retaining ownership of any cancellation cleanup already requested. + await DrainCancelledRunAsync().ConfigureAwait(false); + } + finally + { + runCts?.Dispose(); + runActivity?.Dispose(); + } } - - runCts?.Dispose(); - runActivity?.Dispose(); } } @@ -1477,40 +1524,50 @@ private TimeSpan GetRemainingStartupTimeout(long startupStartTimestamp, TimeSpan return elapsed >= startupTimeout ? TimeSpan.Zero : startupTimeout - elapsed; } - private async Task CancelAppHostStartupAsync(CancellationTokenSource runCancellationTokenSource, Task pendingRun, CancellationToken cancellationToken) + private async Task CancelAppHostRunAsync( + CancellationTokenSource runCancellationTokenSource, + Task pendingRun, + TimeSpan timeout, + CancellationToken cancellationToken) { runCancellationTokenSource.Cancel(); try { - // The timeout is a safety net for the startup-timeout path (no Ctrl+C). When the user - // presses Ctrl+C, cancellationToken fires and WaitAsync exits immediately via the token - // rather than waiting for the full timeout duration. - await pendingRun.WaitAsync(s_appHostStartupCancellationTimeout, _timeProvider, cancellationToken).ConfigureAwait(false); + await pendingRun.WaitAsync(timeout, _timeProvider, cancellationToken).ConfigureAwait(false); } - catch (OperationCanceledException) when (runCancellationTokenSource.IsCancellationRequested || cancellationToken.IsCancellationRequested) + catch (OperationCanceledException) { + // Cancellation of the wait must reach the command's owning cancellation handler. + // Only cancellation of the run task itself is safe to absorb here. + cancellationToken.ThrowIfCancellationRequested(); } - catch (TimeoutException ex) + catch (TimeoutException ex) when (timeout != Timeout.InfiniteTimeSpan) { _logger.LogDebug(ex, "Timed out waiting for AppHost startup cancellation to complete."); - _ = ObserveAppHostRunFailureAsync(pendingRun); + _ = DrainAppHostRunAfterCancellationAsync(pendingRun); } catch (Exception ex) { _logger.LogDebug(ex, "AppHost run failed after startup cancellation."); } + + // Cancellation can race the timeout or a run failure and lose the WaitAsync race. + cancellationToken.ThrowIfCancellationRequested(); } - private async Task ObserveAppHostRunFailureAsync(Task pendingRun) + private async Task DrainAppHostRunAfterCancellationAsync(Task pendingRun) { try { await pendingRun.ConfigureAwait(false); } + catch (OperationCanceledException) + { + } catch (Exception ex) { - _logger.LogDebug(ex, "AppHost run failed after startup cancellation timeout."); + _logger.LogDebug(ex, "AppHost run failed while startup cancellation was being drained."); } } diff --git a/tests/Aspire.Cli.Tests/Backchannel/ExtensionBackchannelTests.cs b/tests/Aspire.Cli.Tests/Backchannel/ExtensionBackchannelTests.cs index 8b61cf226b9..c9f5ff8f9a1 100644 --- a/tests/Aspire.Cli.Tests/Backchannel/ExtensionBackchannelTests.cs +++ b/tests/Aspire.Cli.Tests/Backchannel/ExtensionBackchannelTests.cs @@ -10,8 +10,25 @@ namespace Aspire.Cli.Tests.Backchannel; -public class ExtensionBackchannelTests(ITestOutputHelper outputHelper) +public class ExtensionBackchannelTests(ITestOutputHelper outputHelper) : IDisposable { + private readonly ConsoleCancellationManager _cancellationManager = new(Timeout.InfiniteTimeSpan); + + [Fact] + public async Task StopCliAsync_CancelsTheRunningCommand() + { + using var workspace = TemporaryWorkspace.CreateForCli(outputHelper); + _cancellationManager.SetStartedHandler(Task.CompletedTask); + var rpcTarget = new ExtensionRpcTarget( + new ConfigurationBuilder().Build(), + workspace.CreateExecutionContext(), + _cancellationManager); + + await rpcTarget.StopCliAsync(); + + Assert.True(_cancellationManager.IsCancellationRequested); + } + [Fact] public async Task ConnectAsync_WhenConnectionSetupFails_PropagatesFailureAndAllowsRetry() { @@ -146,7 +163,9 @@ public async Task ConnectAsync_WhenConnectorIsCanceled_ConcurrentWaiterTakesOver Assert.Equal(2, connectAttempts); } - private static ExtensionBackchannel CreateBackchannel( + public void Dispose() => _cancellationManager.Dispose(); + + private ExtensionBackchannel CreateBackchannel( string endpoint, CliExecutionContext executionContext, Func? connectCoreAsyncOverride = null) @@ -159,7 +178,11 @@ private static ExtensionBackchannel CreateBackchannel( }) .Build(); - return new ExtensionBackchannel(NullLogger.Instance, new ExtensionRpcTarget(configuration, executionContext), configuration, connectCoreAsyncOverride); + return new ExtensionBackchannel( + NullLogger.Instance, + new ExtensionRpcTarget(configuration, executionContext, _cancellationManager), + configuration, + connectCoreAsyncOverride); } } diff --git a/tests/Aspire.Cli.Tests/Commands/AppHostLauncherTests.cs b/tests/Aspire.Cli.Tests/Commands/AppHostLauncherTests.cs index 67b134784a8..bbdfaf74b1d 100644 --- a/tests/Aspire.Cli.Tests/Commands/AppHostLauncherTests.cs +++ b/tests/Aspire.Cli.Tests/Commands/AppHostLauncherTests.cs @@ -1745,22 +1745,6 @@ public void Kill(bool entireProcessTree) public ValueTask DisposeAsync() => ValueTask.CompletedTask; } - private sealed class SignalingFakeTimeProvider(TimeSpan signaledDueTime) : FakeTimeProvider - { - public TaskCompletionSource TimerCreated { get; } = new(TaskCreationOptions.RunContinuationsAsynchronously); - - public override ITimer CreateTimer(TimerCallback callback, object? state, TimeSpan dueTime, TimeSpan period) - { - var timer = base.CreateTimer(callback, state, dueTime, period); - if (dueTime == signaledDueTime) - { - TimerCreated.TrySetResult(); - } - - return timer; - } - } - private sealed class FixedLayoutDiscovery : ILayoutDiscovery { public LayoutConfiguration? DiscoverLayout(string? projectDirectory = null) => null; diff --git a/tests/Aspire.Cli.Tests/Commands/RunCommandTests.cs b/tests/Aspire.Cli.Tests/Commands/RunCommandTests.cs index 9fee6788182..22d4d013ebe 100644 --- a/tests/Aspire.Cli.Tests/Commands/RunCommandTests.cs +++ b/tests/Aspire.Cli.Tests/Commands/RunCommandTests.cs @@ -435,6 +435,188 @@ public async Task RunCommand_WhenCancelledDuringBuild_ExitsSuccessfully() Assert.Empty(interactionService.DisplayedErrors); } + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task RunCommand_WhenExtensionStopsCliDuringStartup_AwaitsRunCleanupPastLocalTimeout(bool buildCompleted) + { + using var workspace = TemporaryWorkspace.CreateForCli(outputHelper); + var interactionService = new TestInteractionService(); + var timeProvider = new SignalingFakeTimeProvider(TimeSpan.FromSeconds(5)); + + var appHostDir = workspace.WorkspaceRoot.CreateSubdirectory("AppHost"); + var appHostFile = new FileInfo(Path.Combine(appHostDir.FullName, "AppHost.csproj")); + await File.WriteAllTextAsync(appHostFile.FullName, "", TestContext.Current.CancellationToken); + + var projectLocator = new TestProjectLocator + { + UseOrFindAppHostProjectFileWithBehaviorAsyncCallback = (_, _, _, _) => + Task.FromResult(new AppHostProjectSearchResult(appHostFile, [appHostFile])) + }; + + var projectFactory = new TestAppHostProjectFactory(); + + var services = CliTestHelper.CreateServiceCollection(workspace, outputHelper, options => + { + options.InteractionServiceFactory = _ => interactionService; + options.ProjectLocatorFactory = _ => projectLocator; + options.AppHostProjectFactory = _ => projectFactory; + options.TimeProvider = timeProvider; + }); + + using var provider = services.BuildServiceProvider(); + var command = provider.GetRequiredService(); + var cancellationManager = provider.GetRequiredService(); + var rpcTarget = provider.GetRequiredService(); + var runCompletionSource = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var stopRequested = false; + interactionService.ShowStatusCallback = status => + { + if (buildCompleted && status == RunCommandStrings.ConnectingToAppHost) + { + Assert.True(rpcTarget.StopCliAsync().IsCompletedSuccessfully); + stopRequested = true; + } + }; + projectFactory.RunAsyncCallback = (context, runCancellationToken) => + { + // Cancel synchronously at the selected startup phase so InvokeAsync cannot yield + // until cleanup is waiting on the incomplete run task. + if (buildCompleted) + { + context.BuildCompletionSource!.SetResult(true); + } + else + { + Assert.True(rpcTarget.StopCliAsync().IsCompletedSuccessfully); + Assert.True(runCancellationToken.IsCancellationRequested); + stopRequested = true; + } + return runCompletionSource.Task; + }; + var result = command.Parse($"run --apphost {appHostFile.FullName}"); + var pendingCommand = result.InvokeAsync(cancellationToken: cancellationManager.Token); + + int exitCode; + try + { + Assert.True(stopRequested, "The setup must request cancellation synchronously before inspecting cleanup."); + + // Advance beyond the old local timeout only after the command has entered its cleanup wait. + timeProvider.Advance(TimeSpan.FromSeconds(6)); + + Assert.False(timeProvider.TimerCreated.Task.IsCompleted, "Manager-owned cancellation armed the local startup timeout."); + Assert.False(pendingCommand.IsCompleted, "The CLI exited before run cleanup completed."); + } + finally + { + runCompletionSource.TrySetResult(CliExitCodes.Cancelled); + exitCode = await pendingCommand.DefaultTimeout(); + } + + Assert.Equal(CliExitCodes.Success, exitCode); + Assert.Empty(interactionService.DisplayedErrors); + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task RunCommand_WhenDirectlyCancelledDuringStartup_StopsWaitingAtLocalTimeout(bool buildCompleted) + { + using var workspace = TemporaryWorkspace.CreateForCli(outputHelper); + using var cts = new CancellationTokenSource(); + var interactionService = new TestInteractionService(); + var timeProvider = new SignalingFakeTimeProvider(TimeSpan.FromSeconds(5)); + + var appHostDir = workspace.WorkspaceRoot.CreateSubdirectory("AppHost"); + var appHostFile = new FileInfo(Path.Combine(appHostDir.FullName, "AppHost.csproj")); + await File.WriteAllTextAsync(appHostFile.FullName, "", TestContext.Current.CancellationToken); + + var projectLocator = new TestProjectLocator + { + UseOrFindAppHostProjectFileWithBehaviorAsyncCallback = (_, _, _, _) => + Task.FromResult(new AppHostProjectSearchResult(appHostFile, [appHostFile])) + }; + + var buildStarted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var cleanupStarted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var cleanupCompleted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var cleanupCanFinish = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var connectingToAppHost = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + interactionService.ShowStatusCallback = status => + { + if (status == RunCommandStrings.ConnectingToAppHost) + { + connectingToAppHost.TrySetResult(); + } + }; + var projectFactory = new TestAppHostProjectFactory + { + RunAsyncCallback = async (context, cancellationToken) => + { + if (buildCompleted) + { + context.BuildCompletionSource!.SetResult(true); + } + buildStarted.TrySetResult(); + + try + { + await Task.Delay(Timeout.InfiniteTimeSpan, cancellationToken); + } + catch (OperationCanceledException) + { + cleanupStarted.TrySetResult(); + await cleanupCanFinish.Task; + cleanupCompleted.TrySetResult(); + context.BuildCompletionSource?.TrySetResult(false); + } + + return CliExitCodes.Cancelled; + } + }; + + var services = CliTestHelper.CreateServiceCollection(workspace, outputHelper, options => + { + options.InteractionServiceFactory = _ => interactionService; + options.ProjectLocatorFactory = _ => projectLocator; + options.AppHostProjectFactory = _ => projectFactory; + options.TimeProvider = timeProvider; + }); + + using var provider = services.BuildServiceProvider(); + var command = provider.GetRequiredService(); + var result = command.Parse($"run --apphost {appHostFile.FullName}"); + var pendingCommand = result.InvokeAsync(cancellationToken: cts.Token); + + await buildStarted.Task.DefaultTimeout(); + if (buildCompleted) + { + await connectingToAppHost.Task.DefaultTimeout(); + } + cts.Cancel(); + await cleanupStarted.Task.DefaultTimeout(); + await timeProvider.TimerCreated.Task.DefaultTimeout(); + + try + { + Assert.False(pendingCommand.IsCompleted, "Direct cancellation did not wait for run cleanup."); + timeProvider.Advance(TimeSpan.FromSeconds(6)); + + var exitCode = await pendingCommand.DefaultTimeout(); + + Assert.Equal(CliExitCodes.Success, exitCode); + Assert.False(cleanupCompleted.Task.IsCompleted, "Direct cancellation waited past its local safety timeout."); + Assert.Empty(interactionService.DisplayedErrors); + } + finally + { + cleanupCanFinish.TrySetResult(); + await cleanupCompleted.Task.DefaultTimeout(); + await pendingCommand.DefaultTimeout(); + } + } + [Fact] public async Task RunCommand_WhenProjectReturnsCancelledDuringBuild_ExitsSuccessfully() { @@ -536,65 +718,104 @@ public async Task RunCommand_WhenAppHostStartupTimesOut_DisplaysTimeoutGuidance( Assert.True(runCancellationObserved.Task.IsCompletedSuccessfully); } - [Fact] - public async Task RunCommand_WhenCancelledDuringStartupTimeout_ExitsWithoutWaitingForFullTimeout() + [Theory] + [InlineData(false, false)] + [InlineData(false, true)] + [InlineData(true, false)] + [InlineData(true, true)] + public async Task RunCommand_WhenCancelledDuringStartupTimeoutCleanup_AwaitsRunCleanup(bool managerOwned, bool timeoutWins) { - // Verifies that when Ctrl+C fires (cancellationToken) during startup, the command exits - // promptly rather than blocking for the 5-second CancelAppHostStartupAsync timeout. using var workspace = TemporaryWorkspace.CreateForCli(outputHelper); using var cts = new CancellationTokenSource(); var interactionService = new TestInteractionService(); - var buildCompleted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); - + var timeProvider = new SignalingFakeTimeProvider(TimeSpan.FromSeconds(5)); var appHostDir = workspace.WorkspaceRoot.CreateSubdirectory("AppHost"); var appHostFile = new FileInfo(Path.Combine(appHostDir.FullName, "AppHost.csproj")); - await File.WriteAllTextAsync(appHostFile.FullName, ""); + await File.WriteAllTextAsync(appHostFile.FullName, "", TestContext.Current.CancellationToken); var projectLocator = new TestProjectLocator { UseOrFindAppHostProjectFileWithBehaviorAsyncCallback = (_, _, _, _) => Task.FromResult(new AppHostProjectSearchResult(appHostFile, [appHostFile])) }; - + var runCompletion = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); var projectFactory = new TestAppHostProjectFactory { - RunAsyncCallback = async (context, _) => + RunAsyncCallback = (context, _) => { - context.BuildCompletionSource?.TrySetResult(true); - buildCompleted.SetResult(); - - // Never signal BackchannelCompletionSource and ignore cancellation to - // simulate a hung AppHost process. - await Task.Delay(TimeSpan.FromSeconds(30), CancellationToken.None); - return 0; + context.BuildCompletionSource!.SetResult(true); + return runCompletion.Task; + } + }; + interactionService.ShowStatusCallback = status => + { + if (status == RunCommandStrings.ConnectingToAppHost) + { + // Expire startup before its wait is created, keeping the transition into + // timeout cleanup synchronous rather than depending on scheduler timing. + timeProvider.Advance(TimeSpan.FromSeconds(1)); } }; - var services = CliTestHelper.CreateServiceCollection(workspace, outputHelper, options => { options.InteractionServiceFactory = _ => interactionService; options.ProjectLocatorFactory = _ => projectLocator; options.AppHostProjectFactory = _ => projectFactory; + options.TimeProvider = timeProvider; + options.ConfigurationCallback += config => config[CliConfigNames.AppHostStartupTimeout] = "1"; }); using var provider = services.BuildServiceProvider(); + var cancellationManager = provider.GetRequiredService(); + var rpcTarget = provider.GetRequiredService(); + var cleanupWaits = 0; + timeProvider.TimerCreatedCallback = () => + { + if (++cleanupWaits == 1) + { + if (timeoutWins) + { + // Complete the timeout first, then cancel before the command observes it. + timeProvider.Advance(TimeSpan.FromSeconds(5)); + } + // Interrupt the startup-timeout cleanup wait, not the startup wait itself. + if (managerOwned) + { + Assert.True(rpcTarget.StopCliAsync().IsCompletedSuccessfully); + } + else + { + cts.Cancel(); + } + } + }; var command = provider.GetRequiredService(); - var result = command.Parse($"run --apphost {appHostFile.FullName}"); - - var pendingRun = result.InvokeAsync(cancellationToken: cts.Token); - - // Cancel after build completes to simulate Ctrl+C during startup. - await buildCompleted.Task.DefaultTimeout(); - cts.Cancel(); + var pendingCommand = command.Parse($"run --apphost {appHostFile.FullName}") + .InvokeAsync(cancellationToken: managerOwned ? cancellationManager.Token : cts.Token); - var stopwatch = Stopwatch.StartNew(); - var exitCode = await pendingRun.DefaultTimeout(); - stopwatch.Stop(); + int exitCode; + try + { + Assert.Equal(managerOwned ? 1 : 2, cleanupWaits); + Assert.False(pendingCommand.IsCompleted, "Cancellation escaped the startup-timeout cleanup wait."); + timeProvider.Advance(TimeSpan.FromSeconds(6)); + if (managerOwned) + { + Assert.False(pendingCommand.IsCompleted, "Manager-owned cleanup stopped at the local timeout."); + } + else + { + Assert.Equal(CliExitCodes.Success, await pendingCommand.DefaultTimeout()); + } + } + finally + { + runCompletion.TrySetResult(CliExitCodes.Cancelled); + exitCode = await pendingCommand.DefaultTimeout(); + } - // Without the cancellationToken plumbing, this would block for the full 5-second - // CancelAppHostStartupAsync timeout. With the fix, it exits promptly. - Assert.True(stopwatch.Elapsed < TimeSpan.FromSeconds(3), $"Expected prompt exit after Ctrl+C, but took {stopwatch.Elapsed}."); Assert.Equal(CliExitCodes.Success, exitCode); + Assert.Empty(interactionService.DisplayedErrors); } [Fact] @@ -1725,8 +1946,10 @@ public async Task RunCommand_DetachedEarlyExit_PropagatesExitCodeWithoutUnexpect Assert.DoesNotContain(interactionService.DisplayedErrors, error => error.Contains("An unexpected error occurred", StringComparison.Ordinal)); } - [Fact] - public async Task RunCommand_WhenCancelledDuringStartupRpc_CompletesSuccessfully() + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task RunCommand_WhenCancelledDuringStartupRpc_AwaitsRunCleanup(bool useRunCancellationToken) { using var workspace = TemporaryWorkspace.CreateForCli(outputHelper); using var cts = new CancellationTokenSource(); @@ -1745,7 +1968,7 @@ public async Task RunCommand_WhenCancelledDuringStartupRpc_CompletesSuccessfully var interactionService = new TestInteractionService(); var projectFactory = new TestAppHostProjectFactory { - RunAsyncCallback = async (context, _) => + RunAsyncCallback = async (context, runCancellationToken) => { context.BuildCompletionSource?.TrySetResult(true); context.BackchannelCompletionSource?.TrySetResult(new TestAppHostBackchannel @@ -1753,7 +1976,7 @@ public async Task RunCommand_WhenCancelledDuringStartupRpc_CompletesSuccessfully GetDashboardUrlsAsyncCallback = ct => { cts.Cancel(); - return Task.FromCanceled(ct); + return Task.FromCanceled(useRunCancellationToken ? runCancellationToken : ct); } }); @@ -1773,21 +1996,27 @@ public async Task RunCommand_WhenCancelledDuringStartupRpc_CompletesSuccessfully var command = provider.GetRequiredService(); var result = command.Parse($"run --apphost {appHostFile.FullName}"); + var pendingCommand = result.InvokeAsync(cancellationToken: cts.Token); + int exitCode; try { - var exitCode = await result.InvokeAsync(cancellationToken: cts.Token).DefaultTimeout(); - - Assert.Equal(CliExitCodes.Success, exitCode); - Assert.Empty(interactionService.DisplayedErrors); + Assert.True(cts.IsCancellationRequested, "The dashboard RPC must request cancellation synchronously."); + Assert.False(pendingCommand.IsCompleted, "The CLI exited before run cleanup completed."); } finally { runCanExit.TrySetResult(); + exitCode = await pendingCommand.DefaultTimeout(); } + + Assert.Equal(CliExitCodes.Success, exitCode); + Assert.Empty(interactionService.DisplayedErrors); } - [Fact] - public async Task RunCommand_WhenStartupRpcThrowsUnrelatedCancellationAfterUserCancellation_DoesNotTreatRunAsSuccessful() + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task RunCommand_WhenStartupRpcFailsAfterUserCancellation_AwaitsRunCleanupAndPreservesFailure(bool unrelatedCancellation) { using var workspace = TemporaryWorkspace.CreateForCli(outputHelper); using var cts = new CancellationTokenSource(); @@ -1816,7 +2045,9 @@ public async Task RunCommand_WhenStartupRpcThrowsUnrelatedCancellationAfterUserC { cts.Cancel(); unrelatedCts.Cancel(); - return Task.FromCanceled(unrelatedCts.Token); + return unrelatedCancellation + ? Task.FromCanceled(unrelatedCts.Token) + : Task.FromException(new InvalidOperationException("Dashboard RPC failed.")); } }); @@ -1836,17 +2067,143 @@ public async Task RunCommand_WhenStartupRpcThrowsUnrelatedCancellationAfterUserC var command = provider.GetRequiredService(); var result = command.Parse($"run --apphost {appHostFile.FullName}"); + var pendingCommand = result.InvokeAsync(cancellationToken: cts.Token); + int exitCode; try { - var exitCode = await result.InvokeAsync(cancellationToken: cts.Token).DefaultTimeout(); - - Assert.Equal(CliExitCodes.FailedToDotnetRunAppHost, exitCode); - Assert.Contains(interactionService.DisplayedErrors, error => error.Contains("unexpected error", StringComparison.OrdinalIgnoreCase)); + Assert.True(cts.IsCancellationRequested, "The dashboard RPC must request cancellation synchronously."); + Assert.False(pendingCommand.IsCompleted, "An RPC failure bypassed cancellation cleanup."); } finally { runCanExit.TrySetResult(); + exitCode = await pendingCommand.DefaultTimeout(); + } + + Assert.Equal(CliExitCodes.FailedToDotnetRunAppHost, exitCode); + Assert.Contains(interactionService.DisplayedErrors, error => error.Contains("unexpected error", StringComparison.OrdinalIgnoreCase)); + } + + [Theory] + [InlineData(true, false, false)] + [InlineData(false, false, false)] + [InlineData(true, true, false)] + [InlineData(false, true, false)] + [InlineData(true, false, true)] + public async Task RunCommand_WhenCancelledDuringFinalTeardown_AwaitsRunCleanup(bool managerOwned, bool teardownFails, bool extensionCancelled) + { + using var workspace = TemporaryWorkspace.CreateForCli(outputHelper); + using var cts = new CancellationTokenSource(); + var timeProvider = new SignalingFakeTimeProvider(TimeSpan.FromSeconds(5)); + var runCompletion = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var configurationProvider = new CallbackConfigurationProvider(); + var rpcFailed = false; + var cancellationRequested = false; + var runToken = CancellationToken.None; + if (extensionCancelled) + { + // Expire the initial extension-operation cancellation wait synchronously. + // A later stopCli must upgrade that already-handled local wait to manager ownership. + timeProvider.TimerCreatedCallback = () => timeProvider.Advance(TimeSpan.FromSeconds(5)); } + + var appHostFile = new FileInfo(Path.Combine(workspace.WorkspaceRoot.FullName, "AppHost.csproj")); + await File.WriteAllTextAsync(appHostFile.FullName, "", TestContext.Current.CancellationToken); + var projectLocator = new TestProjectLocator + { + UseOrFindAppHostProjectFileWithBehaviorAsyncCallback = (_, _, _, _) => + Task.FromResult(new AppHostProjectSearchResult(appHostFile, [appHostFile])) + }; + var projectFactory = new TestAppHostProjectFactory + { + RunAsyncCallback = (context, token) => + { + runToken = token; + context.BuildCompletionSource!.SetResult(true); + context.BackchannelCompletionSource!.SetResult(new TestAppHostBackchannel + { + GetDashboardUrlsAsyncCallback = _ => + { + rpcFailed = true; + return Task.FromException(extensionCancelled + ? new ExtensionOperationCanceledException("Extension operation canceled.") + : new InvalidOperationException("Dashboard RPC failed.")); + } + }); + return runCompletion.Task; + } + }; + var services = CliTestHelper.CreateServiceCollection(workspace, outputHelper, options => + { + options.ProjectLocatorFactory = _ => projectLocator; + options.AppHostProjectFactory = _ => projectFactory; + options.TimeProvider = timeProvider; + }); + var originalConfiguration = Assert.IsAssignableFrom( + services.Single(service => service.ServiceType == typeof(IConfiguration)).ImplementationInstance); + services.AddSingleton(new ConfigurationBuilder() + .AddConfiguration(originalConfiguration) + .Add(configurationProvider) + .Build()); + + using var provider = services.BuildServiceProvider(); + var manager = provider.GetRequiredService(); + var rpcTarget = provider.GetRequiredService(); + configurationProvider.Reading = key => + { + if (!rpcFailed || cancellationRequested || key != KnownConfigNames.CliRunDetached) + { + return; + } + + // The final detached-child check used to run after the cancellation snapshot. + // Deliver cancellation at that exact boundary, without a scheduler-dependent delay. + cancellationRequested = true; + if (managerOwned) + { + Assert.True(rpcTarget.StopCliAsync().IsCompletedSuccessfully); + } + else + { + cts.Cancel(); + } + + if (teardownFails) + { + throw new InvalidOperationException("Final teardown failed."); + } + }; + + var command = provider.GetRequiredService(); + var pendingCommand = command.Parse($"run --apphost {appHostFile.FullName}") + .InvokeAsync(cancellationToken: managerOwned ? manager.Token : cts.Token); + int exitCode; + try + { + Assert.True(cancellationRequested); + Assert.True(runToken.IsCancellationRequested); + Assert.False(pendingCommand.IsCompleted, "Final teardown bypassed cancellation cleanup."); + + timeProvider.Advance(TimeSpan.FromSeconds(6)); + Assert.Equal(!managerOwned || extensionCancelled, timeProvider.TimerCreated.Task.IsCompleted); + if (managerOwned) + { + Assert.False(pendingCommand.IsCompleted, "Manager-owned cleanup used a local deadline."); + } + else + { + await pendingCommand.DefaultTimeout(); + Assert.False(runCompletion.Task.IsCompleted); + } + } + finally + { + runCompletion.TrySetResult(CliExitCodes.Cancelled); + exitCode = await pendingCommand.DefaultTimeout(); + } + + Assert.Equal(teardownFails ? CliExitCodes.InvalidCommand : + extensionCancelled ? CliExitCodes.Success : CliExitCodes.FailedToDotnetRunAppHost, exitCode); } [Fact] diff --git a/tests/Aspire.Cli.Tests/TestServices/CallbackConfigurationProvider.cs b/tests/Aspire.Cli.Tests/TestServices/CallbackConfigurationProvider.cs new file mode 100644 index 00000000000..794de59e7de --- /dev/null +++ b/tests/Aspire.Cli.Tests/TestServices/CallbackConfigurationProvider.cs @@ -0,0 +1,19 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.Extensions.Configuration; + +namespace Aspire.Cli.Tests.TestServices; + +internal sealed class CallbackConfigurationProvider : ConfigurationProvider, IConfigurationSource +{ + public Action? Reading { get; set; } + + public override bool TryGet(string key, out string? value) + { + Reading?.Invoke(key); + return base.TryGet(key, out value); + } + + public IConfigurationProvider Build(IConfigurationBuilder builder) => this; +} diff --git a/tests/Aspire.Cli.Tests/TestServices/SignalingFakeTimeProvider.cs b/tests/Aspire.Cli.Tests/TestServices/SignalingFakeTimeProvider.cs new file mode 100644 index 00000000000..29846ef14ac --- /dev/null +++ b/tests/Aspire.Cli.Tests/TestServices/SignalingFakeTimeProvider.cs @@ -0,0 +1,24 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using Microsoft.Extensions.Time.Testing; + +namespace Aspire.Cli.Tests.TestServices; + +internal sealed class SignalingFakeTimeProvider(TimeSpan signaledDueTime) : FakeTimeProvider +{ + public TaskCompletionSource TimerCreated { get; } = new(TaskCreationOptions.RunContinuationsAsynchronously); + public Action? TimerCreatedCallback { get; set; } + + public override ITimer CreateTimer(TimerCallback callback, object? state, TimeSpan dueTime, TimeSpan period) + { + var timer = base.CreateTimer(callback, state, dueTime, period); + if (dueTime == signaledDueTime) + { + TimerCreatedCallback?.Invoke(); + TimerCreated.TrySetResult(); + } + + return timer; + } +} diff --git a/tests/Aspire.Cli.Tests/Utils/CliTestHelper.cs b/tests/Aspire.Cli.Tests/Utils/CliTestHelper.cs index 2183a885d97..e42d379c48b 100644 --- a/tests/Aspire.Cli.Tests/Utils/CliTestHelper.cs +++ b/tests/Aspire.Cli.Tests/Utils/CliTestHelper.cs @@ -598,7 +598,8 @@ public ISolutionLocator CreateDefaultSolutionLocatorFactory(IServiceProvider ser { var configuration = serviceProvider.GetRequiredService(); var executionContext = serviceProvider.GetRequiredService(); - return new ExtensionRpcTarget(configuration, executionContext); + var cancellationManager = serviceProvider.GetRequiredService(); + return new ExtensionRpcTarget(configuration, executionContext, cancellationManager); }; public Func ExtensionBackchannelFactory { get; set; } = serviceProvider =>