diff --git a/src/DiffEngine.Tests/MaxInstanceReplacementTests.cs b/src/DiffEngine.Tests/MaxInstanceReplacementTests.cs
new file mode 100644
index 00000000..7f673f0b
--- /dev/null
+++ b/src/DiffEngine.Tests/MaxInstanceReplacementTests.cs
@@ -0,0 +1,134 @@
+#if NET10_0
+///
+/// Relaunching a pair that is already open does not spend an instance slot.
+///
+/// InnerLaunch killed the tool already showing the pair and then asked MaxInstance whether it was
+/// allowed to launch. Once the per process counter was spent that answer was no - so a re-failing
+/// test closed its own diff window and opened nothing in its place, and reported
+/// TooManyRunningDiffTools with the move sent as processId null. The number of open tools had gone
+/// down, not up.
+///
+///
+[NotInParallel]
+[RunOn(TUnit.Core.Enums.OS.Windows)]
+public class MaxInstanceReplacementTests :
+ IDisposable
+{
+ [Test]
+ public async Task RelaunchingTheSamePairIsNotANewInstance()
+ {
+ LimitTo(1);
+
+ ProcessCleanup.Refresh();
+ var first = DiffRunner.Launch(temp, target);
+ await Assert.That(first).IsEqualTo(LaunchResult.StartedNewInstance);
+
+ await WaitForRunning();
+ ProcessCleanup.Refresh();
+
+ // The slot is spent, but this pair is already open - so this is a replacement
+ var second = DiffRunner.Launch(temp, target);
+ await Assert.That(second).IsEqualTo(LaunchResult.StartedNewInstance);
+ }
+
+ ///
+ /// And a different pair still runs into the limit, which is what the limit is for.
+ ///
+ [Test]
+ public async Task ADifferentPairStillHitsTheLimit()
+ {
+ LimitTo(1);
+
+ ProcessCleanup.Refresh();
+ await Assert.That(DiffRunner.Launch(temp, target)).IsEqualTo(LaunchResult.StartedNewInstance);
+
+ ProcessCleanup.Refresh();
+ await Assert.That(DiffRunner.Launch(otherTemp, otherTarget)).IsEqualTo(LaunchResult.TooManyRunningDiffTools);
+ }
+
+ ///
+ /// Through the environment variable, because that is what MaxInstance reads first and this
+ /// machine may well have one set - DiffEngine_MaxInstances persists per user, so the app
+ /// domain setting alone silently loses to it. Process scoped, so nothing outlives the run.
+ ///
+ static void LimitTo(int value)
+ {
+ Environment.SetEnvironmentVariable(variable, value.ToString());
+ // Forces MaxInstance to re-read, since it caches the first answer
+ DiffRunner.MaxInstancesToLaunch(value);
+ MaxInstance.ResetCount();
+ }
+
+ async Task WaitForRunning()
+ {
+ var command = tool.BuildCommand(temp, target);
+ for (var attempt = 0; attempt < 40; attempt++)
+ {
+ ProcessCleanup.Refresh();
+ if (ProcessCleanup.IsRunning(command))
+ {
+ return;
+ }
+
+ await Task.Delay(250);
+ }
+ }
+
+ public MaxInstanceReplacementTests()
+ {
+ Directory.CreateDirectory(directory);
+
+ temp = Write("first.received.zzmax");
+ target = Write("first.verified.zzmax");
+ otherTemp = Write("second.received.zzmax");
+ otherTarget = Write("second.verified.zzmax");
+ tool = DiffTools.AddTool(
+ name: $"MaxProbe{Guid.NewGuid():N}",
+ autoRefresh: false,
+ isMdi: false,
+ supportsText: false,
+ requiresTarget: true,
+ useShellExecute: false,
+ launchArguments: new(
+ Left: (t, g) => $"\"{t}\" \"{g}\"",
+ Right: (t, g) => $"\"{g}\" \"{t}\""),
+ exePath: FakeDiffTool.Exe,
+ binaryExtensions: [".zzmax"])!;
+ }
+
+ string Write(string name)
+ {
+ var path = Path.Combine(directory, name);
+ File.WriteAllText(path, name);
+ return path;
+ }
+
+ public void Dispose()
+ {
+ Environment.SetEnvironmentVariable(variable, original);
+ DiffRunner.MaxInstancesToLaunch(5);
+ MaxInstance.ResetCount();
+ try
+ {
+ DiffRunner.Kill(temp, target);
+ DiffRunner.Kill(otherTemp, otherTarget);
+ Directory.Delete(directory, true);
+ }
+ catch
+ {
+ // Best effort: the fake tool exits on its own anyway
+ }
+ }
+
+ // Per test, not static: two tests sharing paths means the second one's first launch finds
+ // the first one's tool still open and is treated as a replacement
+ const string variable = "DiffEngine_MaxInstances";
+ string? original = Environment.GetEnvironmentVariable(variable);
+ string directory = Path.Combine(Path.GetTempPath(), $"DiffEngine.MaxInstance.{Guid.NewGuid():N}");
+ ResolvedTool tool;
+ string temp;
+ string target;
+ string otherTemp;
+ string otherTarget;
+}
+#endif
diff --git a/src/DiffEngine/DiffRunner.cs b/src/DiffEngine/DiffRunner.cs
index 2aaab7a4..eb492457 100644
--- a/src/DiffEngine/DiffRunner.cs
+++ b/src/DiffEngine/DiffRunner.cs
@@ -157,6 +157,7 @@ static LaunchResult InnerLaunch(TryResolveTool tryResolveTool, string tempFile,
tool.CommandAndArguments(tempFile, targetFile, out var arguments, out var command);
var canKill = !tool.IsMdi;
+ var replacing = false;
if (ProcessCleanup.TryGetProcessInfo(command, out var processCommand))
{
if (tool.AutoRefresh)
@@ -165,10 +166,14 @@ static LaunchResult InnerLaunch(TryResolveTool tryResolveTool, string tempFile,
return LaunchResult.AlreadyRunningAndSupportsRefresh;
}
- KillIfNotMdi(tool, command);
+ replacing = KillIfNotMdi(tool, command);
}
- if (MaxInstance.Reached())
+ // A replacement does not raise the number of open tools, so it does not spend a slot. The
+ // kill above has already happened by this point, so counting it meant a re-failing test
+ // closed its own window and then declined to open another
+ if (!replacing &&
+ MaxInstance.Reached())
{
DiffEngineTray.AddMove(tempFile, targetFile, tool.ExePath, arguments, canKill, null);
return LaunchResult.TooManyRunningDiffTools;
@@ -192,6 +197,7 @@ static async Task InnerLaunchAsync(TryResolveTool tryResolveTool,
tool.CommandAndArguments(tempFile, targetFile, out var arguments, out var command);
var canKill = !tool.IsMdi;
+ var replacing = false;
if (ProcessCleanup.TryGetProcessInfo(command, out var processCommand))
{
if (tool.AutoRefresh)
@@ -200,10 +206,12 @@ static async Task InnerLaunchAsync(TryResolveTool tryResolveTool,
return LaunchResult.AlreadyRunningAndSupportsRefresh;
}
- KillIfNotMdi(tool, command);
+ replacing = KillIfNotMdi(tool, command);
}
- if (MaxInstance.Reached())
+ // As above: a replacement is not a new instance
+ if (!replacing &&
+ MaxInstance.Reached())
{
await DiffEngineTray.AddMoveAsync(tempFile, targetFile, tool.ExePath, arguments, canKill, null);
return LaunchResult.TooManyRunningDiffTools;
@@ -295,12 +303,19 @@ Failed to launch diff tool.
}
}
- static void KillIfNotMdi(ResolvedTool tool, string command)
+ ///
+ /// Closes the tool already showing this pair, and reports whether it did. An MDI tool hosts
+ /// every diff in one window, so there is nothing to close and nothing being replaced.
+ ///
+ static bool KillIfNotMdi(ResolvedTool tool, string command)
{
- if (!tool.IsMdi)
+ if (tool.IsMdi)
{
- ProcessCleanup.Kill(command);
+ return false;
}
+
+ ProcessCleanup.Kill(command);
+ return true;
}
static void GuardFiles(string tempFile, string targetFile)
diff --git a/src/DiffEngine/MaxInstance.cs b/src/DiffEngine/MaxInstance.cs
index 3a14d6cb..6bd7fb13 100644
--- a/src/DiffEngine/MaxInstance.cs
+++ b/src/DiffEngine/MaxInstance.cs
@@ -45,6 +45,13 @@ public static void SetForUser(int value)
ResetCapturedValue();
}
+ ///
+ /// Forgets the instances launched so far. For tests, which need the limit to mean something
+ /// definite rather than however many launches the rest of the run happened to make.
+ ///
+ internal static void ResetCount() =>
+ Interlocked.Exchange(ref launchedInstances, 0);
+
public static bool Reached()
{
var count = Interlocked.Increment(ref launchedInstances);