diff --git a/src/DiffEngine.Tests/DiffRunnerTests.cs b/src/DiffEngine.Tests/DiffRunnerTests.cs index ed9acacf..b2bbe2ad 100644 --- a/src/DiffEngine.Tests/DiffRunnerTests.cs +++ b/src/DiffEngine.Tests/DiffRunnerTests.cs @@ -181,6 +181,53 @@ public async Task LaunchAndKill() await Assert.That(ProcessCleanup.IsRunning(command)).IsFalse(); } + /// + /// A tool launched after the process list was filled has to be visible without anything + /// asking for a refresh first. + /// + /// The list was filled once, by the static constructor, and nothing in the library ever + /// refreshed it. So within one process a tool launched after first use was invisible for the + /// rest of that process: Kill logged "No matching commands" and left it open, a relaunch + /// never saw the running instance and opened a second window while spending another + /// MaxInstance slot, and an AutoRefresh tool was relaunched rather than reused. Every test + /// here polls through WaitForRunning, which calls Refresh itself, so none of them could see + /// it - and FakeDiffTool exits on its own after five seconds, so even a kill that found + /// nothing looked like a kill that worked. + /// + /// + [Test] + public async Task AToolLaunchedAfterTheListWasFilledIsVisible() + { + await WaitForRunning(false); + // The stale snapshot: taken while the tool is not running + ProcessCleanup.Refresh(); + await Assert.That(ProcessCleanup.IsRunning(command)).IsFalse(); + + var result = await DiffRunner.LaunchAsync(file1, file2); + await Assert.That(result).IsEqualTo(LaunchResult.StartedNewInstance); + + // Nothing calls Refresh from here. Bounded well inside FakeDiffTool's five second life, + // so a pass means the query saw it rather than that the process outlived the poll + await Assert.That(await PollCached()).IsTrue(); + + DiffRunner.Kill(file1, file2); + await WaitForRunning(false); + } + + async Task PollCached() + { + for (var attempt = 0; attempt < 12; attempt++) + { + if (ProcessCleanup.IsRunning(command)) + { + return true; + } + + await Task.Delay(250); + } + + return false; + } [Test] public async Task LaunchAndKillAsync() { diff --git a/src/DiffEngine/Process/ProcessCleanup.cs b/src/DiffEngine/Process/ProcessCleanup.cs index f94324cc..9c61c691 100644 --- a/src/DiffEngine/Process/ProcessCleanup.cs +++ b/src/DiffEngine/Process/ProcessCleanup.cs @@ -27,6 +27,11 @@ static ProcessCleanup() Refresh(); } + /// + /// The processes as of the last . A snapshot, so callers that need to + /// know what is running now go through or , both of + /// which take their own. + /// public static IReadOnlyCollection Commands => commands; [MemberNotNull(nameof(commands))] @@ -60,6 +65,14 @@ public static void Kill(string command) command = TrimCommand(command); } + // The list was filled once by the static constructor and nothing in the library refreshed + // it, so this matched against whatever was running the first time anything touched + // DiffEngine. In one process a Launch followed by a Kill for the same pair logged "No + // matching commands" and left the tool open. It also keeps the PID as fresh as this can + // make it: a process that has since exited may have had its id reused, and terminating + // from a stale snapshot kills whatever holds it now + Refresh(); + var matchingCommands = Commands .Where(_ => _.Command == command).ToList(); Logging.Write($"Kill: {command}. Matching count: {matchingCommands.Count}"); @@ -91,6 +104,11 @@ public static bool TryGetProcessInfo(string command, out ProcessCommand process) command = TrimCommand(command); } + // As for Kill: the question is what is running now. Against the startup snapshot a tool + // launched later was never seen again, so an AutoRefresh tool opened a second window + // instead of being reused and every relaunch spent another MaxInstance slot + Refresh(); + process = commands.FirstOrDefault(_ => _.Command == command); return !process.Equals(default(ProcessCommand)); }