diff --git a/src/AgentOs.RemoteAgent/AgentOs.RemoteAgent.csproj b/src/AgentOs.RemoteAgent/AgentOs.RemoteAgent.csproj index 1147667..9fe4794 100644 --- a/src/AgentOs.RemoteAgent/AgentOs.RemoteAgent.csproj +++ b/src/AgentOs.RemoteAgent/AgentOs.RemoteAgent.csproj @@ -6,6 +6,8 @@ enable enable AgentOs.RemoteAgent + + true diff --git a/src/AgentOs.RemoteAgent/PreviewHost.cs b/src/AgentOs.RemoteAgent/PreviewHost.cs index 751589b..ce87008 100644 --- a/src/AgentOs.RemoteAgent/PreviewHost.cs +++ b/src/AgentOs.RemoteAgent/PreviewHost.cs @@ -37,6 +37,7 @@ private sealed class Running(string id, string repoKey, string dir) public string RepoKey { get; } = repoKey; public string Dir { get; } = dir; public Process? Process { get; set; } + public ProcessJob Job { get; } = ProcessJob.Create(); public CancellationTokenSource Stop { get; } = new(); public Queue Tail { get; } = new(); } @@ -143,11 +144,19 @@ private async Task RunAsync(Running run, PreviewStart start) return; } + // Readiness is "something answers on the port" — so the port must be free before we start, or a leftover + // server (another checkout's dev server) would read as this app being up. + if (await IsListeningAsync(recipe.Url, ct).ConfigureAwait(false)) + { + await FailAsync(run, $"Something else on this computer is already using port {recipe.Url.Port}. Close that app (or stop its preview), then try again.").ConfigureAwait(false); + return; + } await report(run.Id, "Starting", $"Running {recipe.Command}").ConfigureAwait(false); Console.WriteLine($"[agent] preview {run.RepoKey}: {recipe.Command} (in {run.Dir})"); var process = Process.Start(ShellFor(recipe.Command, run.Dir)) ?? throw new InvalidOperationException("The command didn't start."); run.Process = process; + run.Job.Add(process); WritePidFile(run.Dir, process); process.OutputDataReceived += (_, e) => Remember(run, e.Data); process.ErrorDataReceived += (_, e) => Remember(run, e.Data); @@ -178,6 +187,7 @@ private async Task RunAsync(Running run, PreviewStart start) if (_byId.TryRemove(run.Id, out _)) { _idByRepo.TryRemove(new KeyValuePair(run.RepoKey, run.Id)); + run.Job.Dispose(); // whatever it left running in the background goes with it await report(run.Id, "Stopped", $"The app ended (exit {process.ExitCode}).\n{TailOf(run)}").ConfigureAwait(false); } } @@ -314,8 +324,7 @@ private static void WritePidFile(string dir, Process process) private static void Kill(Running run) { - try { run.Process?.Kill(entireProcessTree: true); } - catch (Exception ex) when (ex is InvalidOperationException or System.ComponentModel.Win32Exception or NotSupportedException) { _ = ex; } + run.Job.Dispose(); // the app's whole tree, background children included try { File.Delete(Path.Join(run.Dir, "preview.pid")); } catch (Exception ex) when (ex is IOException or UnauthorizedAccessException) { _ = ex; } } diff --git a/src/AgentOs.RemoteAgent/ProcessJob.cs b/src/AgentOs.RemoteAgent/ProcessJob.cs new file mode 100644 index 0000000..ca8c124 --- /dev/null +++ b/src/AgentOs.RemoteAgent/ProcessJob.cs @@ -0,0 +1,121 @@ +// Everything a run starts dies with the run. A CLI agent's background tools (a `vite` dev server, a test host, a corpus +// converter) outlived their run and held ports and memory for hours; when the runner itself was restarted, the whole +// CLI run carried on orphaned and raced its own retry for the same branch. +// +// On Windows the run goes into a Job Object with KILL_ON_JOB_CLOSE: children join the job automatically, and closing +// the handle — Dispose, or the OS when the runner process dies for any reason — kills every process still in it. +// Elsewhere it falls back to killing the root's current process tree on Dispose (no orphan guarantee after a hard kill). + +using System.ComponentModel; +using System.Diagnostics; +using System.Runtime.InteropServices; + +namespace AgentOs.RemoteAgent; + +/// A process tree that is killed as a whole when disposed (or when the runner exits, on Windows). +internal sealed partial class ProcessJob : IDisposable +{ + private const int JobObjectExtendedLimitInformation = 9; + private const uint KillOnJobClose = 0x2000; + + private nint _handle; + private readonly List _roots = []; + + private ProcessJob(nint handle) => _handle = handle; + + /// A new, empty job. + public static ProcessJob Create() + { + if (!OperatingSystem.IsWindows()) { return new ProcessJob(0); } + var handle = CreateJobObjectW(0, null); + if (handle == 0) { return new ProcessJob(0); } // best effort: still tracks roots + var info = new ExtendedLimitInformation { BasicLimitInformation = new BasicLimitInformation { LimitFlags = KillOnJobClose } }; + if (!SetInformationJobObject(handle, JobObjectExtendedLimitInformation, ref info, (uint)Marshal.SizeOf())) + { + CloseHandle(handle); + return new ProcessJob(0); + } + return new ProcessJob(handle); + } + + /// Put a just-started process (and everything it starts from now on) into the job. + public void Add(Process process) + { + ArgumentNullException.ThrowIfNull(process); + lock (_roots) { _roots.Add(process); } + if (_handle != 0) + { + try { _ = AssignProcessToJobObject(_handle, process.Handle); } + catch (Exception ex) when (ex is InvalidOperationException or Win32Exception) { _ = ex; } // exited already + } + } + + /// Kill everything in the job. + public void Dispose() + { + List roots; + lock (_roots) { roots = [.. _roots]; _roots.Clear(); } + foreach (var root in roots) + { + try { if (!root.HasExited) { root.Kill(entireProcessTree: true); } } + catch (Exception ex) when (ex is InvalidOperationException or Win32Exception or NotSupportedException) { _ = ex; } + } + var handle = Interlocked.Exchange(ref _handle, 0); + if (handle != 0) { CloseHandle(handle); } // KILL_ON_JOB_CLOSE: background children that left the tree go too + } + + [StructLayout(LayoutKind.Sequential)] + private struct BasicLimitInformation + { + public long PerProcessUserTimeLimit; + public long PerJobUserTimeLimit; + public uint LimitFlags; + public nuint MinimumWorkingSetSize; + public nuint MaximumWorkingSetSize; + public uint ActiveProcessLimit; + public nuint Affinity; + public uint PriorityClass; + public uint SchedulingClass; + } + + [StructLayout(LayoutKind.Sequential)] + private struct IoCounters + { + public ulong ReadOperationCount; + public ulong WriteOperationCount; + public ulong OtherOperationCount; + public ulong ReadTransferCount; + public ulong WriteTransferCount; + public ulong OtherTransferCount; + } + + [StructLayout(LayoutKind.Sequential)] + private struct ExtendedLimitInformation + { + public BasicLimitInformation BasicLimitInformation; + public IoCounters IoInfo; + public nuint ProcessMemoryLimit; + public nuint JobMemoryLimit; + public nuint PeakProcessMemoryUsed; + public nuint PeakJobMemoryUsed; + } + + [LibraryImport("kernel32.dll", SetLastError = true, StringMarshalling = StringMarshalling.Utf16)] + [DefaultDllImportSearchPaths(DllImportSearchPath.System32)] + private static partial nint CreateJobObjectW(nint attributes, string? name); + + [LibraryImport("kernel32.dll", SetLastError = true)] + [DefaultDllImportSearchPaths(DllImportSearchPath.System32)] + [return: MarshalAs(UnmanagedType.Bool)] + private static partial bool SetInformationJobObject(nint job, int infoClass, ref ExtendedLimitInformation info, uint length); + + [LibraryImport("kernel32.dll", SetLastError = true)] + [DefaultDllImportSearchPaths(DllImportSearchPath.System32)] + [return: MarshalAs(UnmanagedType.Bool)] + private static partial bool AssignProcessToJobObject(nint job, nint process); + + [LibraryImport("kernel32.dll", SetLastError = true)] + [DefaultDllImportSearchPaths(DllImportSearchPath.System32)] + [return: MarshalAs(UnmanagedType.Bool)] + private static partial bool CloseHandle(nint handle); +} diff --git a/src/AgentOs.RemoteAgent/Program.cs b/src/AgentOs.RemoteAgent/Program.cs index 820fa0f..6207078 100644 --- a/src/AgentOs.RemoteAgent/Program.cs +++ b/src/AgentOs.RemoteAgent/Program.cs @@ -282,6 +282,9 @@ static async Task RunLlmAsync(RemoteExecRequest request, Func< using var process = Process.Start(psi) ?? throw new InvalidOperationException($"Failed to start '{cmd}'. Is the {cli} CLI installed and on PATH?"); + // Whatever the run starts in the background (dev servers, test hosts) ends with it — see ProcessJob.cs. + using var job = AgentOs.RemoteAgent.ProcessJob.Create(); + job.Add(process); RunningCli[request.Id] = process; if (profile.PromptViaStdin) @@ -476,6 +479,8 @@ static async Task ExecuteToolAsync( using var process = Process.Start(psi) ?? throw new InvalidOperationException($"Failed to start shell process."); + using var job = AgentOs.RemoteAgent.ProcessJob.Create(); + job.Add(process); var stdoutTask = process.StandardOutput.ReadToEndAsync(); var stderrTask = process.StandardError.ReadToEndAsync(); diff --git a/tests/AgentOs.Tests/AgentOs.Tests.csproj b/tests/AgentOs.Tests/AgentOs.Tests.csproj index 3182ba3..d478f07 100644 --- a/tests/AgentOs.Tests/AgentOs.Tests.csproj +++ b/tests/AgentOs.Tests/AgentOs.Tests.csproj @@ -2,6 +2,8 @@ AgentOs.Tests + + true AgentOs.Tests false true @@ -53,6 +55,7 @@ + diff --git a/tests/AgentOs.Tests/Sessions/ProcessJobTests.cs b/tests/AgentOs.Tests/Sessions/ProcessJobTests.cs new file mode 100644 index 0000000..cf7a528 --- /dev/null +++ b/tests/AgentOs.Tests/Sessions/ProcessJobTests.cs @@ -0,0 +1,61 @@ +// A run's background children (a dev server it started and left behind) must die with the run — on the lab a vite +// server from a finished run held port 5173 and made "Try the app" report a stranger's server as ready. + +using System; +using System.Diagnostics; +using System.IO; +using System.Threading.Tasks; +using AgentOs.RemoteAgent; +using Shouldly; +using Xunit; + +namespace AgentOs.Tests.Sessions; + +public class ProcessJobTests +{ + [Fact] + public async Task Dispose_KillsABackgroundChildThatOutlivedItsParent() + { + if (!OperatingSystem.IsWindows()) { return; } // the Job Object guarantee is Windows-only (see ProcessJob.cs) + + var pidFile = Path.Join(Path.GetTempPath(), $"agentos-job-{Guid.NewGuid():N}.txt"); + try + { + // The parent starts a detached child, writes its pid, and exits at once — the child is now an orphan. + var psi = new ProcessStartInfo("powershell.exe") + { + UseShellExecute = false, + CreateNoWindow = true, + }; + psi.ArgumentList.Add("-NoProfile"); + psi.ArgumentList.Add("-Command"); + psi.ArgumentList.Add($"$p = Start-Process ping -ArgumentList '-n','120','127.0.0.1' -PassThru -WindowStyle Hidden; Set-Content -Path '{pidFile}' -Value $p.Id"); + + using var job = ProcessJob.Create(); + using (var parent = Process.Start(psi)!) + { + job.Add(parent); + await parent.WaitForExitAsync(); + } + var childPid = int.Parse((await File.ReadAllTextAsync(pidFile)).Trim(), System.Globalization.CultureInfo.InvariantCulture); + using var child = Process.GetProcessById(childPid); + child.HasExited.ShouldBeFalse(); + + job.Dispose(); + + (await Task.Run(() => child.WaitForExit(TimeSpan.FromSeconds(10)))).ShouldBeTrue(); + } + finally + { + File.Delete(pidFile); + } + } + + [Fact] + public void Dispose_Twice_IsHarmless() + { + var job = ProcessJob.Create(); + job.Dispose(); + Should.NotThrow(job.Dispose); + } +}