Skip to content

Do not spend an instance slot on a relaunch - #802

Merged
SimonCropp merged 1 commit into
mainfrom
fix-kill-before-maxinstance
Aug 22, 2026
Merged

Do not spend an instance slot on a relaunch#802
SimonCropp merged 1 commit into
mainfrom
fix-kill-before-maxinstance

Conversation

@SimonCropp

Copy link
Copy Markdown
Member

InnerLaunch killed the tool already showing a pair and then asked MaxInstance
whether it was allowed to launch. Once the per process counter was spent the
answer was no - so a re-failing test closed its own diff window and opened
nothing in its place, reporting TooManyRunningDiffTools with the move sent as
processId null.

The count is meant to bound how many tools are open at once, and a replacement
does not raise that number. KillIfNotMdi now reports whether it actually closed
something, and a launch that is replacing skips the check. An MDI tool closes
nothing, so it is not replacing and still counts.

MaxInstance gains an internal ResetCount, because the counter is process wide
and never reset, so a limit means nothing definite in a test otherwise.

Worth knowing for anyone else writing one of these: MaxInstance reads
DiffEngine_MaxInstances before the app domain value, and that variable persists
per user - it is set to 5 on this machine, which is why the test sets it per
process rather than calling MaxInstancesToLaunch alone. My first version did
call it alone, and silently ran with a limit of 5.

InnerLaunch killed the tool already showing a pair and then asked MaxInstance
whether it was allowed to launch. Once the per process counter was spent the
answer was no - so a re-failing test closed its own diff window and opened
nothing in its place, reporting TooManyRunningDiffTools with the move sent as
processId null.

The count is meant to bound how many tools are open at once, and a replacement
does not raise that number. KillIfNotMdi now reports whether it actually closed
something, and a launch that is replacing skips the check. An MDI tool closes
nothing, so it is not replacing and still counts.

MaxInstance gains an internal ResetCount, because the counter is process wide
and never reset, so a limit means nothing definite in a test otherwise.

Worth knowing for anyone else writing one of these: MaxInstance reads
DiffEngine_MaxInstances before the app domain value, and that variable persists
per user - it is set to 5 on this machine, which is why the test sets it per
process rather than calling MaxInstancesToLaunch alone. My first version did
call it alone, and silently ran with a limit of 5.
@SimonCropp SimonCropp added this to the 20.0.0 milestone Aug 22, 2026
@SimonCropp
SimonCropp merged commit 99728a1 into main Aug 22, 2026
10 checks passed
@SimonCropp
SimonCropp deleted the fix-kill-before-maxinstance branch August 22, 2026 03:01
This was referenced Aug 26, 2026
This was referenced Aug 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

1 participant