From 6c174afc5506d139e9da20af30dac7a8b1344e44 Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Sat, 10 Oct 2026 14:46:21 +1100 Subject: [PATCH 1/2] Give the host-side kill of a WSL tool time to finish Closing a Windows tool from inside WSL runs Windows PowerShell on the host, and shared the five second wait given to the programs that describe the host. A cold PowerShell on a build agent under WSL 1 took longer, so the tool was left open and the wsl 1 job failed. The kill has a thirty second wait of its own, and asks Windows for the tool's processes by image rather than for every process. The tests that run the script no longer hold a thread while PowerShell starts, and take its error stream instead of letting it into the test log. --- claude.md | 2 +- src/DiffEngine.Tests/WslKillScriptTests.cs | 51 +++++++++++++++++----- src/DiffEngine/Wsl/WslHost.cs | 11 ++--- src/DiffEngine/Wsl/WslInterop.cs | 20 +++++++-- 4 files changed, 64 insertions(+), 20 deletions(-) diff --git a/claude.md b/claude.md index 2e37aefd..6f56aca6 100644 --- a/claude.md +++ b/claude.md @@ -829,7 +829,7 @@ the comment there about not caching "nothing staged" asks for. ### Key Patterns - Tool discovery uses wildcard path matching (`WildcardFileFinder`) to find executables in common install locations. A wildcard whose matches are all version-named folders takes the highest version; anything else takes the most recently written -- Inside WSL, which `BuildServerDetector.IsWsl` reports and `Detected` does not count as a build server, a tool with no copy in the distribution is resolved from its Windows definition and started through WSL's interop (`Wsl/`: `WslInterop`, `WslHost`, `WslPaths`; `OsSettingsResolver.TryFindOnHost`). The host's variables come from one `cmd.exe /u /c set`, its drives from `/proc/mounts` and the name of the distribution's share from one `wslpath -w /`. Paths are translated in process, not by `wslpath` per path, because `ResolvedTool.GetArguments` runs for every failing pair, opened or not. A program on the host's PATH is found from one listing of each host directory on it (`WslHost.TryFindOnPath`), a stat on a Windows drive from inside WSL being most of a millisecond. What this side holds for such a tool is WSL's `/init` standing in for it: `ps` lists it as `/init ` (`WslInterop.StripProxy`), ending it does not close a windowed tool, and it ends with the terminal session while the window stays. So `ResolvedTool.CanKill` is false, which is what anyone handed the process id is told, and the library closes the tool itself on the host: `WslInterop.Kill` has Windows PowerShell end the processes of the tool's image whose command line names both files. That is a third of a second, so it is only asked once the stand-in is found by its command, and a tool whose session has gone is neither refreshed nor closed. `WslKillScriptTests` runs the script on Windows, where it can be run. It is started with all three streams redirected and closed (`WslInterop.Start`), or it would hold the pipe `dotnet test` reads. Not offered: the viewer (the host's loopback is not the distribution's under WSL's default networking), a Windows executable that is a `.cmd`, and Vim and Neovim. `DiffEngine_WslWindowsTools=false` turns it off. Most of it is tested with no WSL: `WslHostTests` stand a temp directory in for the drive. What only a distribution can answer is `WslLiveTests`, skipped anywhere `WSL_DISTRO_NAME` is not set: the host read, a file opened by a Windows program through its translated path, and a stand-in tool started, found in `ps` and ended on the host. The `wsl` job in `build.yml` runs it under WSL 1 and 2 (`Vampire/setup-wsl` on a Windows runner), from the tests as built on the runner, so only the .NET runtime is installed inside and no other test class can be run there. Its stand-in is one process, Windows PowerShell: WSL's `/init` for a `cmd.exe` stays until the last program that `cmd.exe` started has gone. Real tools were run by hand in an Ubuntu distribution under WSL 2: Beyond Compare, WinMerge and P4Merge +- Inside WSL, which `BuildServerDetector.IsWsl` reports and `Detected` does not count as a build server, a tool with no copy in the distribution is resolved from its Windows definition and started through WSL's interop (`Wsl/`: `WslInterop`, `WslHost`, `WslPaths`; `OsSettingsResolver.TryFindOnHost`). The host's variables come from one `cmd.exe /u /c set`, its drives from `/proc/mounts` and the name of the distribution's share from one `wslpath -w /`. Paths are translated in process, not by `wslpath` per path, because `ResolvedTool.GetArguments` runs for every failing pair, opened or not. A program on the host's PATH is found from one listing of each host directory on it (`WslHost.TryFindOnPath`), a stat on a Windows drive from inside WSL being most of a millisecond. What this side holds for such a tool is WSL's `/init` standing in for it: `ps` lists it as `/init ` (`WslInterop.StripProxy`), ending it does not close a windowed tool, and it ends with the terminal session while the window stays. So `ResolvedTool.CanKill` is false, which is what anyone handed the process id is told, and the library closes the tool itself on the host: `WslInterop.Kill` has Windows PowerShell end the processes of the tool's image whose command line names both files. That is a third of a second on a machine that has run PowerShell lately and several on one that has not, so it is only asked once the stand-in is found by its command, and it is given thirty seconds rather than the five the host's description gets: a build agent under WSL 1 took longer than five, and the tool was left open. A tool whose session has gone is neither refreshed nor closed. `WslKillScriptTests` runs the script on Windows, where it can be run. It is started with all three streams redirected and closed (`WslInterop.Start`), or it would hold the pipe `dotnet test` reads. Not offered: the viewer (the host's loopback is not the distribution's under WSL's default networking), a Windows executable that is a `.cmd`, and Vim and Neovim. `DiffEngine_WslWindowsTools=false` turns it off. Most of it is tested with no WSL: `WslHostTests` stand a temp directory in for the drive. What only a distribution can answer is `WslLiveTests`, skipped anywhere `WSL_DISTRO_NAME` is not set: the host read, a file opened by a Windows program through its translated path, and a stand-in tool started, found in `ps` and ended on the host. The `wsl` job in `build.yml` runs it under WSL 1 and 2 (`Vampire/setup-wsl` on a Windows runner), from the tests as built on the runner, so only the .NET runtime is installed inside and no other test class can be run there. Its stand-in is one process, Windows PowerShell: WSL's `/init` for a `cmd.exe` stays until the last program that `cmd.exe` started has gone. Real tools were run by hand in an Ubuntu distribution under WSL 2: Beyond Compare, WinMerge and P4Merge - Tool order can be customized via `DiffEngine_ToolOrder` environment variable - `DisabledChecker` respects `DiffEngine_Disabled` env var - `ViewerClient` remembers a port found unowned for ten minutes (`RecheckUnownedAfter`), and the library's telling sends - settle, retire, move, delete, the first inline or diff send - skip the connect while that stands. A refused loopback connection costs two seconds on Windows (firewall stealth mode drops the reset), and a green run settles once per inline verification, which was six minutes for a class of 188 inline tests. Probes (`IsOwned`), the hosts and `InlineQueueClient` always ask and correct the memory; so does `SettleAppliedInline`, being one send per accept. Asking, on Windows, is the operating system's listener table first (`ListenerTable`, shared with `PiperClient`): no listener on the port means nobody to connect to, said without the two seconds, and a listener or a table that cannot be read leaves the connect to answer. So the first telling send of a test process, the launch gate's probe and each of its polls no longer wait to be refused. By port alone, whichever address, since the table is only believed when it says nobody is there. Not for a port that accepted a connection in the last second (`TrustOwnerFor`), because reading the table is reading every connection the machine has, and a run of settles to a live owner would pay more for each than the connect costs. The table is Windows' listeners alone, by `GetExtendedTcpTable` (`ListenerTable.Listeners`), falling back to .NET's list of every connection where that call is not there: 0.32 ms beside 3,000 connections where it was 13. What the table found empty is asked about again after a second (`RecheckUnlistedAfter`), so a tray started after a test process is found; the ten minutes is only for what a connect found. And only a refusal or an unanswered connect is remembered as nobody being there (`NobodyThere`). A viewer launch that failed gives its `MaxInstance` slot back diff --git a/src/DiffEngine.Tests/WslKillScriptTests.cs b/src/DiffEngine.Tests/WslKillScriptTests.cs index d0b5c06e..3674bdfd 100644 --- a/src/DiffEngine.Tests/WslKillScriptTests.cs +++ b/src/DiffEngine.Tests/WslKillScriptTests.cs @@ -18,10 +18,10 @@ public async Task TheProcessNamingBothFilesIsEnded() using var tool = StandIn(temp, target); try { - var ended = Run(WslInterop.KillScript("cmd.exe", temp, target)); + var ended = await Run(WslInterop.KillScript("cmd.exe", temp, target)); await Assert.That(ended).IsEqualTo(1); - await Assert.That(tool.WaitForExit(5000)).IsTrue(); + await Assert.That(await Exited(tool)).IsTrue(); } finally { @@ -39,10 +39,10 @@ public async Task CaseIsIgnored() using var tool = StandIn(temp, target); try { - var ended = Run(WslInterop.KillScript("CMD.EXE", temp.ToUpperInvariant(), target.ToUpperInvariant())); + var ended = await Run(WslInterop.KillScript("CMD.EXE", temp.ToUpperInvariant(), target.ToUpperInvariant())); await Assert.That(ended).IsEqualTo(1); - await Assert.That(tool.WaitForExit(5000)).IsTrue(); + await Assert.That(await Exited(tool)).IsTrue(); } finally { @@ -62,8 +62,8 @@ public async Task AProcessNamingOneFileOrOfAnotherImageIsLeft() using var tool = StandIn(temp, otherTarget); try { - await Assert.That(Run(WslInterop.KillScript("cmd.exe", temp, target))).IsEqualTo(0); - await Assert.That(Run(WslInterop.KillScript("notepad.exe", temp, otherTarget))).IsEqualTo(0); + await Assert.That(await Run(WslInterop.KillScript("cmd.exe", temp, target))).IsEqualTo(0); + await Assert.That(await Run(WslInterop.KillScript("notepad.exe", temp, otherTarget))).IsEqualTo(0); await Assert.That(tool.HasExited).IsFalse(); } finally @@ -101,7 +101,16 @@ static void End(Process tool) } } - static int Run(string script) + /// + /// How many processes the script said it ended. + /// + /// Both streams are read to their ends and nothing is waited on, so no thread is held while + /// PowerShell runs. That is seconds the first time on a build agent, and a thread held for + /// them is one the tests beside this one do not have. The error stream is taken as well + /// because that first run reports its progress there, which went into the test log. + /// + /// + static async Task Run(string script) { var encoded = Convert.ToBase64String(Encoding.Unicode.GetBytes(script)); using var process = Process.Start( @@ -109,10 +118,30 @@ static int Run(string script) { UseShellExecute = false, CreateNoWindow = true, - RedirectStandardOutput = true + RedirectStandardOutput = true, + RedirectStandardError = true })!; - var output = process.StandardOutput.ReadToEnd(); - process.WaitForExit(); - return int.Parse(output.Trim()); + var output = process.StandardOutput.ReadToEndAsync(); + var error = process.StandardError.ReadToEndAsync(); + await Task.WhenAll(output, error); + return int.Parse((await output).Trim()); + } + + /// + /// Whether the stand-in has gone, looked at rather than waited on for the same reason. + /// + static async Task Exited(Process tool) + { + for (var attempt = 0; attempt < 100; attempt++) + { + if (tool.HasExited) + { + return true; + } + + await Task.Delay(100); + } + + return false; } } diff --git a/src/DiffEngine/Wsl/WslHost.cs b/src/DiffEngine/Wsl/WslHost.cs index a8823336..b3995fc3 100644 --- a/src/DiffEngine/Wsl/WslHost.cs +++ b/src/DiffEngine/Wsl/WslHost.cs @@ -13,8 +13,9 @@ namespace DiffEngine; /// class WslHost { - // Each program is run once a process. Long enough for a machine under load, and short enough - // that one which never answers does not hold up the first verification for good + // For the two programs asked what the host looks like, each run once a process. Long enough + // for a machine under load, and short enough that one which never answers does not hold up + // the first verification for good const int timeout = 5000; readonly IReadOnlyDictionary variables; @@ -354,7 +355,7 @@ static bool TryFind(string name, string[] candidates, [NotNullWhen(true)] out st /// /// What a program printed, or null when it did not start, failed, or did not finish in time. /// - internal static string? Run(string file, string arguments, string directory, Encoding encoding) + internal static string? Run(string file, string arguments, string directory, Encoding encoding, int wait = timeout) { using var process = new Process { @@ -383,7 +384,7 @@ static bool TryFind(string name, string[] candidates, [NotNullWhen(true)] out st var output = process.StandardOutput.ReadToEndAsync(); // Read so that a program with a lot to say there is not left waiting on a full pipe var error = process.StandardError.ReadToEndAsync(); - if (!process.WaitForExit(timeout)) + if (!process.WaitForExit(wait)) { try { @@ -399,7 +400,7 @@ static bool TryFind(string name, string[] candidates, [NotNullWhen(true)] out st } if (process.ExitCode != 0 || - !Task.WaitAll([output, error], timeout)) + !Task.WaitAll([output, error], wait)) { return null; } diff --git a/src/DiffEngine/Wsl/WslInterop.cs b/src/DiffEngine/Wsl/WslInterop.cs index 86446c0a..be2de11b 100644 --- a/src/DiffEngine/Wsl/WslInterop.cs +++ b/src/DiffEngine/Wsl/WslInterop.cs @@ -26,6 +26,9 @@ static class WslInterop /// internal const string Variable = "DiffEngine_WslWindowsTools"; + // See Kill + const int killTimeout = 30000; + static readonly Lazy host = new(WslHost.Detect); static readonly ConcurrentDictionary programs = new(StringComparer.Ordinal); @@ -133,6 +136,13 @@ public static int Start(string exePath, string arguments) /// a passing verification with no window pays for a look through a list, as it does for /// any other tool. /// + /// + /// A third of a second is a machine that has run PowerShell lately. One that has not takes + /// several to load what the question needs, and a build agent under WSL 1 took more than + /// five: the wait given to the programs that describe the host. Giving up there left the + /// tool open and said nothing was closed, so this has a wait of its own, long enough for + /// that first run. It is spent only where there is a window to close. + /// /// public static bool Kill(ResolvedTool tool, string tempFile, string targetFile) { @@ -154,7 +164,8 @@ public static bool Kill(ResolvedTool tool, string tempFile, string targetFile) powerShell, $"-NoProfile -NonInteractive -EncodedCommand {encoded}", Path.GetDirectoryName(powerShell)!, - Encoding.UTF8); + Encoding.UTF8, + killTimeout); var closed = int.TryParse(output?.Trim(), out var count) && count > 0; Logging.Write($"Kill on the Windows host: {command}. Closed: {closed}"); return closed; @@ -171,13 +182,16 @@ public static bool Kill(ResolvedTool tool, string tempFile, string targetFile) /// /// internal static string KillScript(string image, string tempFile, string targetFile) => + // The image is asked for in the query, so Windows hands back the tool's processes and + // not every process on the machine with its command line. In a query a backslash and a + // quote are each written behind a backslash $$""" function Decode($value) { [Text.Encoding]::Unicode.GetString([Convert]::FromBase64String($value)) } $image = Decode '{{Encode(image)}}' $temp = Decode '{{Encode(tempFile)}}' $target = Decode '{{Encode(targetFile)}}' - $found = @(Get-CimInstance Win32_Process | Where-Object { - $_.Name -eq $image -and + $name = $image.Replace('\', '\\').Replace("'", "\'") + $found = @(Get-CimInstance Win32_Process -Filter "Name = '$name'" | Where-Object { $_.CommandLine -and $_.CommandLine.IndexOf($temp, [StringComparison]::OrdinalIgnoreCase) -ge 0 -and $_.CommandLine.IndexOf($target, [StringComparison]::OrdinalIgnoreCase) -ge 0 From fabe8bac99fb9c1e91a6a49d642ceecde679365c Mon Sep 17 00:00:00 2001 From: Simon Cropp Date: Sat, 10 Oct 2026 15:29:03 +1100 Subject: [PATCH 2/2] Upload the test reports when the Windows job fails A test that fails on time alone cannot be explained from the log, which says how long it took and nothing about what ran beside it. The report has when every test started and ended. --- .github/workflows/build.yml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 7c59339f..0e4e7123 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -63,6 +63,18 @@ jobs: if-no-files-found: ignore retention-days: 14 + # The log names a failing test and how long it took, and nothing about what ran beside it. + # A test that fails on time alone, because the process was stalled for a quarter of a + # minute, cannot be explained from that. The report has when every test started and ended. + - name: Upload test reports on failure + if: failure() + uses: actions/upload-artifact@v7 + with: + name: test-reports-windows + path: '**/TestResults/*-report.html' + if-no-files-found: ignore + retention-days: 14 + # DE0001 warns when a RID has no native renderer. Shipping a package that cannot run on a # platform it claims to support is worse than failing the release. - name: Verify every RID has a native renderer