Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions src/Aspire.Cli/Processes/WindowsConsoleProcessJob.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,11 @@ namespace Aspire.Cli.Processes;
/// <para>
/// The job is also configured with <c>JOB_OBJECT_LIMIT_BREAKAWAY_OK</c> so that DCP (and
/// anything else that needs to outlive the CLI for its own cleanup) can opt out by spawning
/// itself with <c>CREATE_BREAKAWAY_FROM_JOB</c>. Our job only grants permission to break
/// away — the breakaway itself is the responsibility of the spawning code.
/// itself with <c>CREATE_BREAKAWAY_FROM_JOB</c>, provided every nested job also permits
/// breakaway. .NET AppHosts launched through <c>dotnet run</c> do not use this job because
/// its process reaper creates a nested job that does not permit breakaway. Directly launched
/// .NET AppHosts can use it because they have no intervening job. Our job only grants permission
/// to break away — the breakaway itself is the responsibility of the spawning code.
/// </para>
/// <para>
/// IMPORTANT: do NOT dispose this service during the normal shutdown ladder. The graceful
Expand Down
6 changes: 5 additions & 1 deletion src/Aspire.Cli/Projects/DotNetAppHostProject.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1552,7 +1552,10 @@ void SignalBuildCompletion()
// package add, layout, and other short-lived invocations leave these unset so
// they continue to use the shared ladder's force-kill mode.
IsolateConsole = true,
KillOnParentExit = true,
// dotnet run nests its own non-breakaway job inside the CLI job, preventing DCP
// from escaping the CLI job to finish resource cleanup. Direct launches have no
// intervening job and opt back into the CLI job below.
KillOnParentExit = false,
GracefulShutdownSignaler = _gracefulShutdownSignaler,
ShutdownService = _shutdownService,
// The bundled AppHost run hook delegates dotnet run to aspire run. The SDK passes
Expand Down Expand Up @@ -1616,6 +1619,7 @@ void SignalBuildCompletion()
// caller may reuse runOptions for other invocations.
var directRunOptions = runOptions.Clone();
directRunOptions.AppHostArgumentStartIndex = directRun.AppHostArgumentStartIndex;
directRunOptions.KillOnParentExit = true;

return await _runner.RunAppHostCommandAsync(
effectiveAppHostFile,
Expand Down
186 changes: 185 additions & 1 deletion tests/Aspire.Cli.Tests/Processes/WindowsConsoleProcessJobTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,11 @@
using System.Runtime.InteropServices;
using System.Runtime.Versioning;
using Aspire.Cli.Processes;
using Aspire.Cli.Tests.TestServices;

namespace Aspire.Cli.Tests.Processes;

public class WindowsConsoleProcessJobTests
public class WindowsConsoleProcessJobTests(ITestOutputHelper outputHelper)
{
[Fact]
[SupportedOSPlatform("windows")]
Expand Down Expand Up @@ -117,6 +118,189 @@ public void SpawnProcess_WithJob_AssignsChildToJobAtomicallyAtCreation()
}
}

[Fact]
[SupportedOSPlatform("windows")]
public async Task DotNetRunCleanupChild_SurvivesOnlyWhenOuterJobIsOmitted()
{
Assert.SkipUnless(RuntimeInformation.IsOSPlatform(OSPlatform.Windows), "Windows-only test.");

using var workspace = TemporaryWorkspace.CreateForCli(outputHelper);
var projectDirectory = workspace.CreateDirectory("cleanup-probe");
var projectFile = new FileInfo(Path.Combine(projectDirectory.FullName, "CleanupProbe.csproj"));
await File.WriteAllTextAsync(projectFile.FullName, """
<Project Sdk="Microsoft.NET.Sdk">
<PropertyGroup>
<OutputType>Exe</OutputType>
<TargetFramework>net11.0</TargetFramework>
<ImplicitUsings>enable</ImplicitUsings>
</PropertyGroup>
</Project>
""");
await File.WriteAllTextAsync(Path.Combine(projectDirectory.FullName, "Program.cs"), """
using System.Diagnostics;
using System.Globalization;

var markerPath = args[0];
var pidPath = args[1];
var releasePath = args[2];
if (args is [_, _, _, "cleanup"])
{
await File.WriteAllTextAsync(pidPath, Environment.ProcessId.ToString(CultureInfo.InvariantCulture));
while (!File.Exists(releasePath))
{
await Task.Delay(20);
}

await File.WriteAllTextAsync(markerPath, "cleanup complete");
return;
}

var childStartInfo = new ProcessStartInfo(Environment.ProcessPath!)
{
UseShellExecute = false,
};
childStartInfo.ArgumentList.Add(markerPath);
childStartInfo.ArgumentList.Add(pidPath);
childStartInfo.ArgumentList.Add(releasePath);
childStartInfo.ArgumentList.Add("cleanup");

using var child = Process.Start(childStartInfo)
?? throw new InvalidOperationException("Failed to start cleanup child.");
Comment thread
danegsta marked this conversation as resolved.
Resolved
""");

await BuildCleanupProbeAsync(projectFile);

await RunCleanupProbeAsync(projectFile, workspace, assignOuterJob: false, expectCleanup: true);
await RunCleanupProbeAsync(projectFile, workspace, assignOuterJob: true, expectCleanup: false);
}

private static async Task BuildCleanupProbeAsync(FileInfo projectFile)
{
var startInfo = new ProcessStartInfo(ProcessTestHelpers.GetDotNetExecutablePath())
{
WorkingDirectory = projectFile.DirectoryName!,
RedirectStandardOutput = true,
RedirectStandardError = true,
UseShellExecute = false,
};
startInfo.ArgumentList.Add("build");
startInfo.ArgumentList.Add(projectFile.FullName);
startInfo.ArgumentList.Add("--nologo");

using var process = Process.Start(startInfo) ?? throw new InvalidOperationException("Failed to build cleanup probe.");
var stdoutTask = process.StandardOutput.ReadToEndAsync();
var stderrTask = process.StandardError.ReadToEndAsync();
try
{
await process.WaitForExitAsync().WaitAsync(TimeSpan.FromMinutes(1));
var stdout = await stdoutTask;
var stderr = await stderrTask;

Assert.True(
process.ExitCode == 0,
$"Cleanup probe build failed with exit code {process.ExitCode}.{Environment.NewLine}{stdout}{Environment.NewLine}{stderr}");
}
finally
{
if (!process.HasExited)
{
process.Kill(entireProcessTree: true);
await process.WaitForExitAsync();
}
}
}

[SupportedOSPlatform("windows")]
private static async Task RunCleanupProbeAsync(
FileInfo projectFile,
TemporaryWorkspace workspace,
bool assignOuterJob,
bool expectCleanup)
{
var runName = assignOuterJob ? "with-outer-job" : "without-outer-job";
var markerPath = Path.Combine(workspace.Path, $"{runName}.complete");
var pidPath = Path.Combine(workspace.Path, $"{runName}.pid");
var releasePath = Path.Combine(workspace.Path, $"{runName}.release");
var cleanupPid = 0;
using WindowsConsoleProcessJob? outerJob = assignOuterJob ? new WindowsConsoleProcessJob() : null;

using var nulHandle = WindowsProcessInterop.CreateFileW(
"NUL",
WindowsProcessInterop.GenericRead | WindowsProcessInterop.GenericWrite,
WindowsProcessInterop.FileShareRead | WindowsProcessInterop.FileShareWrite,
nint.Zero,
WindowsProcessInterop.OpenExisting,
0,
nint.Zero);

Assert.False(nulHandle.IsInvalid);
Assert.True(WindowsProcessInterop.SetHandleInformation(
nulHandle,
WindowsProcessInterop.HandleFlagInherit,
WindowsProcessInterop.HandleFlagInherit));

var nulRawHandle = nulHandle.DangerousGetHandle();
var stdio = new WindowsProcessInterop.StdioHandles(
Stdin: nulRawHandle,
Stdout: nulRawHandle,
Stderr: nulRawHandle);

var pi = WindowsProcessInterop.SpawnProcess(
ProcessTestHelpers.GetDotNetExecutablePath(),
["run", "--project", projectFile.FullName, "--no-build", "--no-restore", "--", markerPath, pidPath, releasePath],
projectFile.DirectoryName!,
stdio,
environment: null,
createNewConsole: true,
outerJob?.Handle);

var runProcessExited = false;
try
{
using var runProcess = Process.GetProcessById(pi.dwProcessId);
await runProcess.WaitForExitAsync().WaitAsync(TimeSpan.FromSeconds(30));
runProcessExited = true;
Assert.Equal(0, runProcess.ExitCode);

cleanupPid = await ProcessTestHelpers.WaitForProcessIdAsync(pidPath, TestContext.Current.CancellationToken)
.WaitAsync(TimeSpan.FromSeconds(10));
Assert.False(ProcessTestHelpers.IsProcessExited(cleanupPid));

outerJob?.Dispose();

if (expectCleanup)
{
await File.WriteAllTextAsync(releasePath, string.Empty, TestContext.Current.CancellationToken);
await ProcessTestHelpers.WaitForFileAsync(markerPath, TestContext.Current.CancellationToken)
.WaitAsync(TimeSpan.FromSeconds(15));
Assert.True(
ProcessTestHelpers.WaitForProcessExit(cleanupPid, TimeSpan.FromSeconds(15)),
"The cleanup child did not exit after writing its completion marker.");
Assert.Equal("cleanup complete", await File.ReadAllTextAsync(markerPath, TestContext.Current.CancellationToken));
}
else
{
Assert.True(
ProcessTestHelpers.WaitForProcessExit(cleanupPid, TimeSpan.FromSeconds(15)),
"The outer kill-on-close job did not terminate the cleanup child.");
Assert.False(File.Exists(markerPath));
}
}
finally
{
if (!runProcessExited)
{
ProcessTestHelpers.TryKillProcess(pi.dwProcessId);
}
WindowsProcessInterop.CloseHandle(pi.hProcess);
WindowsProcessInterop.CloseHandle(pi.hThread);
if (cleanupPid != 0)
{
ProcessTestHelpers.TryKillProcess(cleanupPid);
}
}
}

[SupportedOSPlatform("windows")]
private static Process SpawnJobAssignedChildProcess(WindowsConsoleProcessJob job, bool createNewConsole)
{
Expand Down
18 changes: 18 additions & 0 deletions tests/Aspire.Cli.Tests/Projects/AppHostServerSessionTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,21 @@ public async Task Start_DoesNotMutateCallerEnvironmentVariables()
Assert.Equal(session.AuthenticationToken, project.ReceivedEnvironmentVariables[KnownConfigNames.RemoteAppHostToken]);
}

[Fact]
public async Task Start_WithIsolatedConsole_RequestsKillOnParentExitForAppHostServer()
{
var project = new RecordingAppHostServerProject();

await using var session = CreateSession(
project,
CancellationToken.None,
isolateConsole: true);
await session.StartAsync();

Assert.True(project.ReceivedRunControl?.IsolateConsole);
Assert.True(project.ReceivedRunControl?.KillOnParentExit);
}

[Fact]
public async Task Start_PropagatesProfilingContextToServerEnvironment()
{
Expand Down Expand Up @@ -586,6 +601,8 @@ private sealed class RecordingAppHostServerProject : IAppHostServerProject

public Dictionary<string, string>? ReceivedEnvironmentVariables { get; private set; }

public AppHostServerRunControl? ReceivedRunControl { get; private set; }

public IProcessExecution? StartedExecution { get; private set; }

public string GetInstanceIdentifier() => AppDirectoryPath;
Expand All @@ -608,6 +625,7 @@ public async Task<AppHostServerRunResult> RunAsync(
ReceivedEnvironmentVariables = environmentVariables is null
? null
: new Dictionary<string, string>(environmentVariables);
ReceivedRunControl = runControl;

var startInfo = new ProcessStartInfo("dotnet")
{
Expand Down
4 changes: 4 additions & 0 deletions tests/Aspire.Cli.Tests/Projects/DotNetAppHostProjectTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,8 @@ public async Task RunAsync_SingleFileAppHostWithoutRunJsonPassesDevelopmentEnvir
Assert.True(noBuild);
Assert.True(noRestore);
Assert.False(options.NoLaunchProfile);
Assert.True(options.IsolateConsole);
Assert.False(options.KillOnParentExit);
Assert.Equal("Development", env![KnownAspNetCoreConfigNames.DotNetEnvironment]);
Assert.False(env.ContainsKey(KnownAspNetCoreConfigNames.Environment));
Assert.Equal("https://localhost:17193;http://localhost:15069", env[KnownAspNetCoreConfigNames.Urls]);
Expand Down Expand Up @@ -1063,6 +1065,8 @@ public async Task RunAsync_ProjectAppHostUsesDirectCommandLaunchAndAppliesLaunch
Assert.Equal(appHostCommand.FullName, command);
Assert.Equal(runWorkingDirectory.FullName, workingDirectory.FullName);
Assert.False(options.NoLaunchProfile);
Assert.True(options.IsolateConsole);
Assert.True(options.KillOnParentExit);
Assert.Equal(
["--from-msbuild", "two words", "--explicit", "1"],
args);
Expand Down
28 changes: 28 additions & 0 deletions tests/Aspire.Cli.Tests/TestServices/ProcessTestHelpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,26 @@ namespace Aspire.Cli.Tests.TestServices;

internal static class ProcessTestHelpers
{
public static string GetDotNetExecutablePath()
{
// Use the same muxer as the test runner. Falling back to PATH can combine a system dotnet
// with repo-local MSBuild environment variables and fail before the process behavior under
// test is reached.
var dotnetHostPath = Environment.GetEnvironmentVariable("DOTNET_HOST_PATH");
if (!string.IsNullOrWhiteSpace(dotnetHostPath) && File.Exists(dotnetHostPath))
{
return dotnetHostPath;
}

var dotnetRoot = Environment.GetEnvironmentVariable("DOTNET_ROOT");
if (!string.IsNullOrWhiteSpace(dotnetRoot))
{
return Path.Combine(dotnetRoot, OperatingSystem.IsWindows() ? "dotnet.exe" : "dotnet");
}

return "dotnet";
}

public static async Task<int> WaitForProcessIdAsync(string pidFile, CancellationToken cancellationToken)
{
while (true)
Expand All @@ -33,6 +53,14 @@ public static async Task<int> WaitForProcessIdAsync(string pidFile, Cancellation
}
}

public static async Task WaitForFileAsync(string path, CancellationToken cancellationToken)
{
while (!File.Exists(path))
{
await Task.Delay(20, cancellationToken);
}
}

public static bool WaitForProcessExit(int pid, TimeSpan timeout)
{
var deadline = DateTime.UtcNow + timeout;
Expand Down
Loading