[Installer] Offer to end running Cockatrice immediately when auto-close fails (#7675)

* [Installer] Offer to end running Cockatrice immediately when auto-close fails

* [Installer] Keep the way out that costs nothing in the first prompt

The prompt now offers to end Cockatrice, so what the deleted close-again text
said has to survive in it: an instance the probe reports as waiting for an
answer to a question of its own has been brought forward, and answering that
window is the way out that ends nothing. Say so, and say where to go when Yes
leaves it listed - Task Manager - so an end that cannot take effect does not
loop on the same dialog.

* [Installer] Bring the comments back in line with the code

GuardCloseTried no longer counts closes, and the file header, CloseHolders and
GuardAppsNotRunning still described an escalation after a second close, as did
the probe's --end-holders note.

* [Installer] End Oracle and Servatrice too when the guard's end answer is confirmed

The guard's end answer only ever stopped cockatrice.exe, so an Oracle holding
the card database or a Servatrice service kept the payload locked and the
prompt came back with nothing left that could free it.

The probe now ends every program the payload installs, services included:
isPayloadProgram() - the list restart_manager_holders already used for its
parent walk - decides the set, so a scanner holding a DLL or a program the
user keeps in the install directory is still named and left to the user. A
service is terminated rather than sent a stop through the SCM, which notices
the process exit and marks it stopped afterwards.

The guard prompt now says what ending costs: an unsaved deck, an unwritten
setting, players on a running Servatrice, and an Oracle mid-way through the
card database. It also keeps the notification-area hint, which the probe's
"has no window to ask" answer depends on, and drops the claim that Oracle
and Servatrice had been brought forward.

* [Installer] Give every running-program prompt the same end-or-retry answers

Four prompts meet a running program and each offered a different set: the
guard ended it, the launcher wait only kept waiting, and the recovery prompt
after a failed purge could only ask Cockatrice to close. With Oracle or
Servatrice holding the files, that last one had no answer that could free
anything - Yes and No both retried the purge and Cancel aborted an install
whose old version was already gone.

All four now follow the guard's shape: one automatic attempt, then a prompt
whose Yes ends the programs it names and whose No retries, with Cancel to
give up.

- RecoverFromHolders closes once and purges before it asks anything, then
  offers EndHolders - the same end answer as the guard - instead of a close
  the user had to trigger himself.
- WaitForLauncherToExit can end the launcher by pid, the one process it
  holds a pid for, rather than looping 60 s waits with Cancel as the only
  way out.
- END_COST and AFTER_CLOSING_GUIDANCE are !defines, so the cost of ending
  and the guidance for the answers that end nothing read the same in every
  prompt.

* [Installer] End the launcher through the handle it was waited on

The "end it" answer used to look the launcher's pid up again after a dialog that
can stay open as long as the user takes, so a pid that had been reused in the
meantime - or a /PID= that was never the launcher - would be terminated instead.
The handle the wait already holds names that process and cannot be handed to
another, so the terminate answer now acts on it.

PROCESS_TERMINATE is requested along with PROCESS_SYNCHRONIZE; when only the
synchronize right is granted the launcher is still waited on and the answer falls
back to asking it to close, instead of reporting a live launcher as gone and
writing the payload over it.

* [Installer] Say what the end answer ends, and report a failed end

The prompt offered "end every program listed above", but EndHolders only ends the
programs the payload installs: a scanner holding a payload DLL, or a service the
Restart Manager names by its long name, was listed and then silently left alone, so
Yes looked like it did nothing and the same dialog came back unchanged.

The wording now says which programs Yes ends and that anything else has to be closed
by hand, and the round after an end that did not free the files says so, so the same
dialog no longer reappears word for word. The three prompts are one macro so they
cannot drift apart again.

The added sentences eat into the 1024 characters one NSIS string holds, so the
holder line's budget drops from 400 to 320 characters to keep the longest message
(cancel line included) inside it: 977 characters, measured against the worst case
the probe can produce.

---------

Co-authored-by: sentry[bot] <39604003+sentry[bot]@users.noreply.github.com>
Co-authored-by: Lukas Brübach <Bruebach.Lukas@bdosecurity.de>

cmake/NSIS.template.in

@@ -42,16 +42,27 @@ Var HolderFound
 ; whether the payload is free, while the abort dialog reports what survived.
 Var GuardExit
 Var GuardOutput
-; How many closes the guard has asked for: 0 before the automatic one, 1 after it, and one
+; Whether the guard has already asked Cockatrice to close by itself: 0 before the automatic
-; more for every "Yes" at the prompt. From the second one on, the prompt offers to end the
+; close, 1 after it. The prompt that follows offers to end the instance rather than asking
-; instance instead (see GuardAppsNotRunning).
+; a second time (see GuardAppsNotRunning).
 Var GuardCloseTried
+; Set when the user answered Yes to ending the programs and the files were still in use
+; afterwards, so the prompt that follows says the end did not help instead of reappearing
+; word for word (see CockEndPrompt).
+Var GuardEndFailed
+; The question CockEndPrompt assembles: one sentence in it is conditional, and the result
+; has to stay inside the characters one NSIS string holds.
+Var GuardPrompt
 Var WaitPid
 Var LauncherExited
 ; How long one WaitForSingleObject on the launcher's handle waits before asking the user
 ; (see WaitForLauncherToExit). A variable because the first slice is short and the ones
 ; after the first question are not.
 Var WaitSlice
+; Whether that handle carries PROCESS_TERMINATE as well as SYNCHRONIZE, and can therefore
+; be ended from the answer the dialog offers. Cleared when only the synchronize right could
+; be had (see WaitForLauncherToExit).
+Var WaitCanEnd
 Var InstallLogPath
 Var LogHandle
 Var LogMessage
@@ -304,10 +315,10 @@ ${EndIf}
 ;
 ; Then make sure no *other* application instance is still running (and holding
 ; file locks) before the previous version is uninstalled. The probe names them
-; and the user is asked to close them; nothing is ever force-stopped, because
+; and the user is asked to close them; nothing is force-stopped without that
-; Cockatrice may hold an unsaved deck and Oracle writes the card database in
+; answer, because Cockatrice may hold an unsaved deck and Oracle writes the card
-; place. On /R the old version is about to be removed, so an answer the probe
+; database in place. On /R the old version is about to be removed, so an answer
-; cannot give cancels the upgrade before it starts rather than after it.
+; the probe cannot give cancels the upgrade before it starts rather than after it.
 ; Only needed this early for /R, where AutoUninstallIfNeeded runs below; for
 ; interactive installs the section re-checks after the directory page, when
 ; $INSTDIR is the directory the user actually picked.
@@ -794,13 +805,49 @@ FunctionEnd
 ;
 ; Nothing is force-stopped without the user asking for it: Cockatrice may be holding
 ; an unsaved deck or showing a "really leave this game?" confirmation, Oracle writes
-; the card database in place and killing it mid-write would corrupt it, and Servatrice
+; the card database in place and can be killed mid-write, and Servatrice may be serving
-; has no window to ask. A close is a request (WM_CLOSE), made automatically once
+; a game. A close is a request (WM_CLOSE), made automatically once because that is what
-; because that is what the user started the installer to do, and again whenever
+; the user started the installer to do, and only Cockatrice can answer one - the other
-; they ask for it from the prompt. When it is not answered the user is told which
+; two have no window. When the close is not answered the user is told which program is
-; program is in the way, and once a close has been asked for twice without freeing
+; in the way and is offered to end all three themselves - the last resort, with what it
-; the files they are offered to end Cockatrice themselves - the last resort, with
+; costs spelled out in the prompt.
-; what it costs spelled out, and never applied to Oracle or Servatrice.
+
+; One sentence for every prompt whose Yes ends the whole set, so that answer reads the same
+; wherever the user meets it: this guard before anything is replaced, the uninstaller's, and
+; the recovery prompt after a failed purge.
+!define END_COST "An unsaved deck, a running game or an unwritten setting is lost, Servatrice drops its players, and an Oracle that is mid-write can damage the card database."
+
+; The other half of that answer - what is left to do when nothing is ended - is shared by
+; every prompt that names the holders, for the same reason.
+!define AFTER_CLOSING_GUIDANCE "Answer the confirmation Cockatrice may be showing - open it from the notification area if it is hidden there - and use Task Manager for a program that would not end."
+
+; usage: set the variable named by the second argument to the probe's listing, then call
+; this. Sets $GuardPrompt to the question the guard, the uninstaller's guard and the
+; recovery path all ask.
+;
+; One macro rather than three copies, because the wording is a promise: Yes ends the
+; programs the payload installs - which is exactly the set EndHolders acts on - and anything
+; else the probe named has to be closed by hand, so a listing that includes a scanner or a
+; service hosted elsewhere does not come with an answer that cannot do what it says.
+;
+; The assembled string has to fit the characters one NSIS string holds, which is what
+; bounds HOLDER_LINE_CHAR_MAX in tools\installer-holder-probe\holder_line.h: with the
+; longest listing that can arrive and the sentence only a failed end adds, it stays inside
+; that budget instead of being cut off at the tail, where the Cancel line is.
+;
+; @param header the sentence that introduces the listing, with its trailing colon
+; @param listing the variable holding the probe's line, $GuardOutput or $HolderOutput
+; @param cancelWord what Cancel gives up, "installation" or "uninstallation"
+!macro CockEndPrompt header listing cancelWord
+    StrCpy $GuardPrompt "${header}$\r$\n$\r$\n${listing}"
+    ; Said only after the previous answer was Yes: without it the same dialog reappears word
+    ; for word, which is what "nothing happened" looked like - the end can fail on a process
+    ; the installer may not open, and it never touches a program that is not one of ours.
+    ${If} $GuardEndFailed = 1
+        StrCpy $GuardPrompt "$GuardPrompt$\r$\n$\r$\nEnding them did not free the files."
+    ${EndIf}
+    StrCpy $GuardPrompt "$GuardPrompt$\r$\n$\r$\nChoose Yes to end the Cockatrice, Oracle and Servatrice programs listed above. ${END_COST}$\r$\n$\r$\nChoose No to try again after closing anything else listed above by hand. ${AFTER_CLOSING_GUIDANCE}$\r$\n$\r$\nChoose Cancel to cancel the ${cancelWord}."
+!macroend
 
 ; Emitted once for the installer and once for the uninstaller: the uninstaller
 ; cannot call installer functions, and its own copy of the probe keeps a running
@@ -840,7 +887,7 @@ FunctionEnd
 ; replaced, plus the processes running from the directory. tools\installer-holder-probe
 ; closes that set by pid, which is why nothing here has to match an executable path. Only
 ; cockatrice.exe is asked: Oracle writes the card database in place and Servatrice has no
-; window, so both stay the user's job, as the prompt says.
+; window to ask, so both are left alone here and only ended by the explicit answer.
 ;
 ; Each instance is brought to the foreground before WM_CLOSE is posted to its main window.
 ; WM_CLOSE is answered, not obeyed - Cockatrice asks before it quits while a card database
@@ -852,9 +899,9 @@ FunctionEnd
 ; log as it stands: it names every holder, including the ones left to the user, so a close
 ; that did nothing can be told apart from one that was never sent.
 ;
-; Never a force-close here: the application decides whether to quit. Ending it is a separate
+; Never a force-close here: the application decides whether to quit. Ending is a separate
-; answer the prompt offers once a close has been asked for twice and ignored, spelled out
+; answer the guard prompt offers when a close did not free the files, spelled out with what
-; with what it costs - see EndHolders - and it is only ever applied to cockatrice.exe.
+; it costs - see EndHolders - and it ends every program the payload installs.
 Function ${prefix}CloseHolders
     Call ${prefix}ExtractHolderProbe
     System::Call 'kernel32::SetEnvironmentVariable(t, t) i ("COCKATRICE_GUARD_DIR", "$GuardDir")'
@@ -871,13 +918,14 @@ Function ${prefix}CloseHolders
 FunctionEnd
 
 ; usage: set $GuardDir to the directory being replaced, then call this. Ends the running
-; Cockatrice instances the probe names.
+; Cockatrice, Oracle and Servatrice instances the probe names.
 ;
 ; This is the only place anything is stopped without being asked to stop, so it is only
 ; reached after the user picked it from a prompt that says what it costs: a client that is
-; still running may have an unsaved deck, a game or a settings write in flight. The probe
+; still running may have an unsaved deck, a game or a settings write in flight, Oracle may
-; ends cockatrice.exe and nothing else - Oracle writes the card database in place and
+; be part-way through writing the card database, and Servatrice may be serving a game. The
-; Servatrice is a service, so both stay the user's job, exactly as they do for the close.
+; probe ends the three programs the payload installs, services included, and nothing else:
+; a scanner or a program the user keeps in the directory is named and left to the user.
 Function ${prefix}EndHolders
     Call ${prefix}ExtractHolderProbe
     System::Call 'kernel32::SetEnvironmentVariable(t, t) i ("COCKATRICE_GUARD_DIR", "$GuardDir")'
@@ -888,7 +936,7 @@ Function ${prefix}EndHolders
     Pop $R4
     Push "end-holders: probe exit=$R3 [$R4]"
     Call ${prefix}CockLogWrite
-    DetailPrint "Ending Cockatrice: $R4"
+    DetailPrint "Ending Cockatrice, Oracle and Servatrice: $R4"
 FunctionEnd
 !macroend
 
@@ -902,9 +950,9 @@ FunctionEnd
 ; the client was still shutting down - never turns into a question. The prompt is what
 ; remains when the close cannot reach everything: Oracle writes the card database in
 ; place, Servatrice has no window, and Cockatrice may still be waiting for an answer in
-; its own confirmation. It offers one more close (Yes), a plain re-check for after the
+; its own confirmation. It offers to end the programs the payload installs - and only those,
-; user closed them by hand (No), and Cancel. Once a close has been asked for twice without
+; which the text says - with what that costs (Yes), a plain re-check for after the user
-; freeing the files, it offers to end Cockatrice instead - the last resort, see EndHolders.
+; closed things by hand (No), and Cancel - ending is the last resort, see EndHolders.
 ;
 ; An error is always shown, in every mode. A /S install without /R has nobody at
 ; the keyboard, but this installer does not promise unattended runs: when an
@@ -914,6 +962,7 @@ FunctionEnd
 Function GuardAppsNotRunning
     !insertmacro CockLog "guard: checking $GuardDir (launcherExited=$LauncherExited)"
     StrCpy $GuardCloseTried 0
+    StrCpy $GuardEndFailed 0
     guard_check:
     Call RunHolderProbe
     ${If} $GuardExit == "0"
@@ -936,45 +985,31 @@ Function GuardAppsNotRunning
             Goto guard_check
         ${EndIf}
         DetailPrint "The install directory is in use by: $GuardOutput"
-        ; A close that has been asked for twice and changed nothing is where asking again
+        ; The automatic close already ran once. Asking Cockatrice to close again is unlikely
-        ; stops being a plan: the instance is either waiting on a question the user has not
+        ; to help: an instance that did not answer the first one is usually sitting on a
-        ; answered, or stuck where a close cannot reach it. Offer to end it - the only thing
+        ; confirmation of its own, and the close probe leaves such an instance alone so that a
-        ; left that can free the files - with the cost in the text, because a client that is
+        ; second WM_CLOSE cannot hide the window the question is in - which is how a pre-3.1.2
-        ; still running may have an unsaved deck, a game or a settings write in flight.
+        ; update used to leave a headless process behind. An instance with no window has
-        ${If} $GuardCloseTried >= 2
+        ; nothing to ask at all. Ending is what is left that can free the files, so the prompt
-            MessageBox MB_YESNO|MB_ICONEXCLAMATION|MB_DEFBUTTON2 \
+        ; offers it here, with what it costs in the text: an unsaved deck, a game, a settings
-                "Cockatrice is still running after being asked to close twice.$\r$\n$\r$\nChoose Yes to end it now - an unsaved deck, a running game or a setting it has not written yet will be lost - or No to ask it to close again.$\r$\n$\r$\n$GuardOutput" \
+        ; write, and an Oracle that may be mid-way through the card database. The answers that
-                IDYES guard_end IDNO guard_close
+        ; end nothing stay reachable from the same dialog: the instance's own confirmation,
-            !insertmacro CockLog "guard: cancelled by user"
+        ; and closing anything by hand.
-            StrCpy $AbortReason \
+        !insertmacro CockEndPrompt "Cockatrice, Oracle or Servatrice is still using files of the installation:" $GuardOutput "installation"
-                "The installation was cancelled because Cockatrice, Oracle or Servatrice is still running and holding files of the previous version.$\r$\n$\r$\nClose them and run the installer again."
+        MessageBox MB_YESNOCANCEL|MB_ICONEXCLAMATION|MB_DEFBUTTON2 "$GuardPrompt" IDYES guard_end IDNO guard_check
-            Call AbortInstall
-            ; Only reached through Yes: AbortInstall does not return.
-            guard_end:
-            !insertmacro CockLog "guard: ending cockatrice on request, then checking again"
-            Call EndHolders
-            ; Back to the ordinary prompt afterwards: Oracle and Servatrice are never ended,
-            ; so what is left may still have to be closed by hand.
-            StrCpy $GuardCloseTried 1
-            Goto guard_check
-        ${EndIf}
-        ; Three answers, and the close is one of them: the automatic attempt above ran
-        ; before this prompt and could not have answered Cockatrice's own confirmation, so a
-        ; second close is often exactly what is needed once the user has dealt with that
-        ; window. NSIS does not let a MessageBox relabel its buttons, so the text says what
-        ; Yes, No and Cancel do.
-        MessageBox MB_YESNOCANCEL|MB_ICONEXCLAMATION|MB_DEFBUTTON1 \
-            "Cockatrice, Oracle or Servatrice is still using files of the installation:$\r$\n$\r$\n$GuardOutput$\r$\n$\r$\nChoose Yes to close Cockatrice and check again. It may ask you to confirm quitting - it does while a card database update is running, a game is open or a deck is unsaved - so answer that window. If it has no window on screen, open it from the notification area first.$\r$\n$\r$\nClose Oracle, Servatrice and any other program listed above by hand. Choose No to check again after that, or Cancel to cancel the installation." \
-            IDYES guard_close IDNO guard_check
         !insertmacro CockLog "guard: cancelled by user"
         StrCpy $AbortReason \
             "The installation was cancelled because Cockatrice, Oracle or Servatrice is still running and holding files of the previous version.$\r$\n$\r$\nClose them and run the installer again."
         Call AbortInstall
         ; Only reached through Yes: AbortInstall does not return.
-        guard_close:
+        guard_end:
-        IntOp $GuardCloseTried $GuardCloseTried + 1
+        !insertmacro CockLog "guard: ending the payload programs on request, then checking again"
-        !insertmacro CockLog "guard: closing cockatrice on request, then checking again"
+        Call EndHolders
-        Call CloseHolders
+        ; Everything the payload installs is ended, but a program that could not be opened
+        ; (another account, a protected process) survives, and a program that is not one of
+        ; ours is never touched at all, so what is left may still have to be closed by hand.
+        ; Go back to the same prompt, which says the end did not free the files.
+        StrCpy $GuardEndFailed 1
         Goto guard_check
     ${EndIf}
     ; Not an answer at all: the probe was not configured, the Restart Manager
@@ -1081,17 +1116,24 @@ Function WaitForLauncherToExit
         !insertmacro CockLog "wait-launcher: no pid given, falling back to name check"
         Return
     ${EndIf}
-    ; PROCESS_SYNCHRONIZE is the only access this needs - enough to wait on the
-    ; handle, and the least likely to be refused for a reason that has nothing to
-    ; do with the upgrade.
-    ;
     ; System.dll maps its output registers so that .rN writes $N and .RN writes
     ; $RN. The caller reads $R0/$R1, so the uppercase .RN form is required; the
     ; original .r0/.r1 wrote $0/$1 and left $R1 empty, which made the "did not
     ; exit" test below compare "" against "0" and abort on every /PID= upgrade.
-    ; Reopened on every Retry below, so a launcher that exited in the meantime answers
+    ; Reopened on every round below, so a launcher that exited in the meantime answers
     ; "already exited" instead of being waited on again.
     ;
+    ; PROCESS_TERMINATE rides along with PROCESS_SYNCHRONIZE so that the "end it"
+    ; answer below acts on this very handle: the pid looked up again after a dialog
+    ; that can stay open as long as the user takes would name whatever process holds
+    ; that pid by then, and a handle cannot be handed to another process.
+    ;
+    ; When those rights are refused - a launcher started by another account, a
+    ; protected process - the wait still has to happen, so the ask is repeated with
+    ; synchronize access alone and $WaitCanEnd records that ending is not on offer.
+    ; Reporting "already exited" instead would be the worse answer: the payload would
+    ; be written over a live process, which is what this wait exists to prevent.
+    ;
     ; Ten seconds, not the full grace: a client that started an update exits in well
     ; under a second, so at ten seconds it is either flushing a card database or - far
     ; more often - sitting on its own "quit anyway?" confirmation with nobody looking at
@@ -1101,7 +1143,8 @@ Function WaitForLauncherToExit
     StrCpy $WaitSlice 10000
     wait_launcher_open:
     ClearErrors
-    System::Call 'kernel32::OpenProcess(i 0x00100000, i 0, i $WaitPid) p.R0'
+    StrCpy $WaitCanEnd 1
+    System::Call 'kernel32::OpenProcess(i 0x00100001, i 0, i $WaitPid) p.R0'
     ; OpenProcess returns a null handle for a process that is already gone. The
     ; System plugin does not raise the NSIS error flag from a called function's
     ; return value, so ${Errors} alone never saw that case and the null handle was
@@ -1110,9 +1153,18 @@ Function WaitForLauncherToExit
     ${If} ${Errors}
         StrCpy $R0 0
     ${EndIf}
+    ${If} $R0 == 0
+        StrCpy $WaitCanEnd 0
+        ClearErrors
+        System::Call 'kernel32::OpenProcess(i 0x00100000, i 0, i $WaitPid) p.R0'
+        ${If} ${Errors}
+            StrCpy $R0 0
+        ${EndIf}
+    ${EndIf}
     ${If} $R0 == 0
     ; The launcher is gone - OpenProcess cannot open a dead process - which is
-    ; exactly the state the wait was for.
+    ; exactly the state the wait was for. A handle refused for want of the terminate
+    ; right is not this case; the synchronize-only ask above covers it.
         !insertmacro CockLog "wait-launcher: already exited"
         DetailPrint "The application that started this installer has already exited."
         StrCpy $LauncherExited 1
@@ -1121,26 +1173,42 @@ Function WaitForLauncherToExit
     DetailPrint "Waiting for the application that started this installer to exit ..."
     ; WAIT_OBJECT_0 is 0; WAIT_TIMEOUT is 258 and a failed wait reports 0xFFFFFFFF.
     System::Call 'kernel32::WaitForSingleObject(p R0, i $WaitSlice) i.R1'
-    System::Call 'kernel32::CloseHandle(p R0)'
     ${If} $R1 != 0
-        !insertmacro CockLog "wait-launcher: did not exit within $WaitSlice ms (wait=$R1)"
+    ; $R0 is deliberately still open: it is the process that timed out, and the answer
+    ; below has to act on that process and not on whatever $WaitPid names by then.
+        !insertmacro CockLog "wait-launcher: did not exit within $WaitSlice ms (wait=$R1, canEnd=$WaitCanEnd)"
         DetailPrint "The application that started this installer did not exit in time."
-        ; Two answers, and "carry on anyway" is not one of them. A live process has its
+        ; Same three answers as every other prompt that waits on a running program, and
-        ; image and its Qt DLLs mapped, so writing the payload over them is the broken
+        ; "carry on anyway" is not one of them: a live process has its image and its Qt
-        ; install this wait exists to prevent - and the guard that runs next names the
+        ; DLLs mapped, so writing the payload over them is the broken install this wait
-        ; same process by its image path and stops there anyway, so an "ignore" would
+        ; exists to prevent. Yes ends the launcher, No asks it to close again and keeps
-        ; only put the same question a moment later, after a close attempt nobody asked
+        ; waiting, Cancel cancels - the launcher is the client the user started this update
-        ; for. Waiting again is what helps: the client is most likely showing its own
+        ; from, and ending it is the same offer the guard makes a moment later.
-        ; "quit anyway?" confirmation, and that window is the thing to answer.
+        MessageBox MB_YESNOCANCEL|MB_ICONEXCLAMATION|MB_DEFBUTTON2 \
-        MessageBox MB_RETRYCANCEL|MB_ICONEXCLAMATION|MB_DEFBUTTON1 \
+            "Cockatrice has not finished closing, so the installation is paused rather than replace files it is still using.$\r$\n$\r$\nIt may be waiting for you to answer its own question about an unsaved deck, an open game or a card database update.$\r$\n$\r$\nChoose Yes to end it now - an unsaved deck, a running game or an unwritten setting is lost.$\r$\n$\r$\nChoose No to bring it to the front, ask it to close again and keep waiting, or Cancel to cancel the installation." \
-            "Cockatrice has not finished closing, so the installation is paused rather than replace files it is still using.$\r$\n$\r$\nIt may be waiting for you to answer its own question about an unsaved deck, an open game or a card database update. Answer that window - choose Retry to bring it to the front, ask it to close again and keep waiting. If it does not answer at all, close it from Task Manager first.$\r$\n$\r$\nChoose Retry to wait again, or Cancel to cancel the installation." \
+            IDYES wait_launcher_end IDNO wait_launcher_retry
-            IDRETRY wait_launcher_retry
         !insertmacro CockLog "wait-launcher: cancelled by user"
         StrCpy $AbortReason \
             "The application that started this update has not exited yet, so the installation was cancelled rather than replace files it is still using.$\r$\n$\r$\nPlease close Cockatrice and run the installer again."
         Call AbortInstall
-        ; Only reached through Retry: AbortInstall does not return.
+        ; Only reached through Yes or No: AbortInstall does not return.
+        wait_launcher_end:
+        ; End it through the handle waited on above, and through nothing else: no pid is
+        ; looked up again, so a process that inherited the number in the meantime - or a
+        ; /PID= that was not the launcher to begin with - cannot be hit by this.
+        !insertmacro CockLog "wait-launcher: ending the launcher on request"
+        ${If} $WaitCanEnd == 0
+            !insertmacro CockLog "wait-launcher: the launcher could not be opened to end it, asking it to close instead"
+            Goto wait_launcher_retry
+        ${EndIf}
+        System::Call 'kernel32::TerminateProcess(p R0, i 1) i.R3'
+        System::Call 'kernel32::CloseHandle(p R0)'
+        StrCpy $R0 0
+        !insertmacro CockLog "wait-launcher: ending the launcher returned $R3"
+        Goto wait_launcher_open
         wait_launcher_retry:
+        System::Call 'kernel32::CloseHandle(p R0)'
+        StrCpy $R0 0
         ; The question has been asked and answered once; from here the wait is the full
         ; grace again, so a slow flush or a confirmation being answered does not turn
         ; into a stream of prompts.
@@ -1148,11 +1216,13 @@ Function WaitForLauncherToExit
         ; The same request the window's close button makes, and the same close the guard
         ; would send a moment later. It brings the client forward as well, which is the
         ; part that helps when the confirmation is behind something else. Nothing is
-        ; force-stopped: a client that is waiting for an answer keeps the last word.
+        ; force-stopped here: the answer that does that is Yes.
         StrCpy $GuardDir $INSTDIR
         Call CloseHolders
         Goto wait_launcher_open
     ${EndIf}
+    System::Call 'kernel32::CloseHandle(p R0)'
+    StrCpy $R0 0
     !insertmacro CockLog "wait-launcher: launcher exited"
     DetailPrint "The application that started this installer has exited."
     StrCpy $LauncherExited 1
@@ -1513,19 +1583,25 @@ FunctionEnd
 
 ; Called after a purge left files behind and LogStaleRuntimeDiagnostics named who is
 ; holding them. Instead of aborting straight away - which on /R strands the user, since
-; the old version is already uninstalled - ask, close what this installer owns, purge
+; the old version is already uninstalled - it closes once, purges again, and only then
-; again, and retry while the user keeps asking. Sets $StaleRuntimeAfterPurge to the last
+; asks: end what is still holding the files, or try the removal again by hand. Sets
-; purge's verdict, so the caller still aborts when the files survive.
+; $StaleRuntimeAfterPurge to the last purge's verdict, so the caller still aborts when the
+; files survive.
 ;
-; Cockatrice is asked to close (WM_CLOSE, the same request as clicking the window's close
+; The shape is the guard's, deliberately: one automatic attempt at a graceful close, then a
-; button) and keeps the last word: it may be waiting for an answer to its own "really
+; prompt whose Yes ends the payload programs and whose No retries. A client that was still
-; quit?" confirmation (a running card database update, an open game, an unsaved deck), and
+; shutting down releases the files in the automatic round and never sees a question, and an
-; that stays its decision. The instance is brought forward first, so that confirmation is
+; Oracle or Servatrice - which a close cannot reach - is what the prompt exists for, so the
-; on screen rather than behind this installer's prompt. Oracle and Servatrice have no
+; user is never left with a dialog whose answers cannot free anything.
-; window to send a close to at all, and nothing here is force-stopped: Oracle writes the
+;
-; card database in place and killing it mid-write would corrupt it. Everything the probe
+; The automatic close is a request (WM_CLOSE) and the client keeps the last word: it may be
-; named that this installer cannot close - those two included - is left to the user, whom
+; waiting for an answer to its own "really quit?" confirmation (a running card database
-; the prompt names it to.
+; update, an open game, an unsaved deck), and that stays its decision. The instance is
+; brought forward first, so that confirmation is on screen rather than behind this
+; installer's prompt.
+;
+; Ending is the same last resort the guard offers, with the same cost spelled out, and only
+; ever the programs the payload installs - see EndHolders.
 ;
 ; Restart Manager's RmShutdown was the alternative and was rejected: it shuts down
 ; services as well as applications, and the only way to bring them back is RmRestart,
@@ -1534,33 +1610,37 @@ FunctionEnd
 ; Always offered, in every mode: a /S install without /R is the only case without someone
 ; at the keyboard, and this installer does not promise unattended runs - if an error needs
 ; a decision, it asks for one. No offer when the probe named nobody: there is nothing to
-; close, and the abort's per-file diagnostics already record what Windows said.
+; end, and the abort's per-file diagnostics already record what Windows said.
 ;
-; Every round offers all three answers: Yes closes Cockatrice and purges again, No purges
+; Every round offers all three answers: Yes ends the programs the payload installs - and
-; again without asking it to close (for when the user closed things by hand), and Cancel
+; says so, because anything else the probe named is left to the user - and purges again, No
-; gives up. The close stays on offer after one that did nothing because the usual reason is
+; purges again without ending anything (for when the user closed things by hand), and
-; Cockatrice's own confirmation still waiting for an answer - the probe waits for that, the
+; Cancel gives up. Each round re-reads the holders, so the prompt names what survived the
-; user can answer it in the application's window, and the next round can then collect the
+; last attempt rather than repeating the first listing.
-; process the earlier attempt could not. Each round re-reads the holders, so the prompt
-; names what survived the last attempt rather than repeating the first listing.
 Function RecoverFromHolders
     ${If} $HolderFound != 1
         !insertmacro CockLog "recover-holders: no holder was named, nothing to offer"
         Return
     ${EndIf}
-    rfh_prompt:
+    StrCpy $GuardEndFailed 0
-    MessageBox MB_YESNOCANCEL|MB_ICONEXCLAMATION|MB_DEFBUTTON1 \
+    ; The automatic attempt, the same one the guard makes before it asks anything.
-        "Files of the previous version of Cockatrice are still in use by:$\r$\n$\r$\n$HolderOutput$\r$\n$\r$\nChoose Yes to close Cockatrice and try the removal again. It may ask you to confirm quitting - it does while a card database update is running, a game is open or a deck is unsaved - so answer that window. Close Oracle, Servatrice and any other program listed above by hand.$\r$\n$\r$\nChoose No to try again after closing the programs yourself, or Cancel to cancel the installation." \
+    !insertmacro CockLog "recover-holders: closing cockatrice, then trying the removal again"
-        IDYES rfh_close IDNO rfh_retry
-    !insertmacro CockLog "recover-holders: declined by user"
-    Return
-    rfh_close:
-    !insertmacro CockLog "recover-holders: closing cockatrice on request, holders were [$HolderOutput]"
-    ; The close is a request, not a fact: CloseHolders waits for the instances it asked to
-    ; exit, so a confirmation of Cockatrice's that is still up is reported as still running
-    ; instead of being measured as a successful close.
     StrCpy $GuardDir $INSTDIR
     Call CloseHolders
+    Goto rfh_retry
+    rfh_prompt:
+    ; The same question the guard asks, so the two cannot drift apart in what they promise.
+    !insertmacro CockEndPrompt "Files of the previous version of Cockatrice are still in use by:" $HolderOutput "installation"
+    MessageBox MB_YESNOCANCEL|MB_ICONEXCLAMATION|MB_DEFBUTTON2 "$GuardPrompt" IDYES rfh_end IDNO rfh_retry
+    !insertmacro CockLog "recover-holders: cancelled by user"
+    Return
+    ; Only reached through Yes: Return does not come back here.
+    rfh_end:
+    !insertmacro CockLog "recover-holders: ending the payload programs on request, then trying the removal again"
+    Call EndHolders
+    ; The next prompt says the end did not free the files, instead of naming the same
+    ; survivors word for word.
+    StrCpy $GuardEndFailed 1
     rfh_retry:
     Call RetryDllPurge
     StrCpy $StaleRuntimeAfterPurge $R0
@@ -1572,7 +1652,7 @@ Function RecoverFromHolders
         Return
     ${EndIf}
     ; Still stuck: refresh the names, so the next prompt and the log describe what
-    ; survived the close and not what was there before it.
+    ; survived the last attempt and not what was there before it.
     Call LogStaleRuntimeDiagnostics
     ${If} $HolderFound != 1
         Return
@@ -1591,6 +1671,7 @@ FunctionEnd
 
 Function un.GuardAppsNotRunning
     StrCpy $GuardCloseTried 0
+    StrCpy $GuardEndFailed 0
     un_guard_check:
     !insertmacro CockLogUn "uninst-guard: checking $GuardDir"
     Call un.RunHolderProbe
@@ -1609,35 +1690,27 @@ Function un.GuardAppsNotRunning
             Goto un_guard_check
         ${EndIf}
         DetailPrint "The install directory is in use by: $GuardOutput"
-        ; Same escalation as in the installer's guard: after a second close that changed
+        ; Same approach as in the installer's guard: the automatic close already ran once, and
-        ; nothing, offer to end the instance, with what it costs in the text.
+        ; asking again is unlikely to help an instance that is sitting on its own confirmation
-        ${If} $GuardCloseTried >= 2
+        ; or has no window a close could reach. Ending is what is left that can free the files,
-            MessageBox MB_YESNO|MB_ICONEXCLAMATION|MB_DEFBUTTON2 \
+        ; so the prompt offers it here, with what it costs in the text, while keeping the
-                "Cockatrice is still running after being asked to close twice.$\r$\n$\r$\nChoose Yes to end it now - an unsaved deck, a running game or a setting it has not written yet will be lost - or No to ask it to close again.$\r$\n$\r$\n$GuardOutput" \
+        ; answers that end nothing - the instance's own question, and closing anything by hand
-                IDYES un_guard_end IDNO un_guard_close
+        ; - reachable from the same dialog. CockEndPrompt words it, so this copy and the
-            !insertmacro CockLogUn "uninst-guard: cancelled by user"
+        ; installer's promise the same thing.
-            SetErrorLevel 1
+        !insertmacro CockEndPrompt "Cockatrice, Oracle or Servatrice is still using files of the installation:" $GuardOutput "uninstallation"
-            Abort
+        MessageBox MB_YESNOCANCEL|MB_ICONEXCLAMATION|MB_DEFBUTTON2 "$GuardPrompt" IDYES un_guard_end IDNO un_guard_check
-            ; Only reached through Yes: Abort does not return.
-            un_guard_end:
-            !insertmacro CockLogUn "uninst-guard: ending cockatrice on request, then checking again"
-            Call un.EndHolders
-            StrCpy $GuardCloseTried 1
-            Goto un_guard_check
-        ${EndIf}
-        ; Same three answers as the installer's guard; the automatic close above already
-        ; ran once before this prompt.
-        MessageBox MB_YESNOCANCEL|MB_ICONEXCLAMATION|MB_DEFBUTTON1 \
-            "Cockatrice, Oracle or Servatrice is still using files of the installation:$\r$\n$\r$\n$GuardOutput$\r$\n$\r$\nChoose Yes to close Cockatrice and check again. It may ask you to confirm quitting - it does while a card database update is running, a game is open or a deck is unsaved - so answer that window. If it has no window on screen, open it from the notification area first.$\r$\n$\r$\nClose Oracle, Servatrice and any other program listed above by hand. Choose No to check again after that, or Cancel to cancel the uninstallation." \
-            IDYES un_guard_close IDNO un_guard_check
         !insertmacro CockLogUn "uninst-guard: cancelled by user"
         SetErrorLevel 1
         Abort
         ; Only reached through Yes: Abort does not return.
-        un_guard_close:
+        un_guard_end:
-        IntOp $GuardCloseTried $GuardCloseTried + 1
+        !insertmacro CockLogUn "uninst-guard: ending the payload programs on request, then checking again"
-        !insertmacro CockLogUn "uninst-guard: closing cockatrice on request, then checking again"
+        Call un.EndHolders
-        Call un.CloseHolders
+        ; Everything the payload installs is ended, but a program that could not be opened
+        ; (another account, a protected process) survives, and a program that is not one of
+        ; ours is never touched at all, so what is left may still have to be closed by hand.
+        ; Go back to the same prompt, which says the end did not free the files.
+        StrCpy $GuardEndFailed 1
         Goto un_guard_check
     ${EndIf}
     ; Same reasoning as in the installer: without an answer from the probe we cannot tell
@@ -1950,9 +2023,9 @@ ${If} $StaleRuntimeAfterPurge = 1
     Call LogStaleRuntimeDiagnostics
     ${If} $HolderFound = 1
         ; On /R the previous version is already uninstalled at this point, so an abort
-        ; leaves the user with no working Cockatrice. Offer to close and purge again,
+        ; leaves the user with no working Cockatrice. It closes once, then offers to end
-        ; repeating while the user keeps saying Yes; this updates
+        ; what is left or to try the removal again; this updates $StaleRuntimeAfterPurge
-        ; $StaleRuntimeAfterPurge with the last retry's verdict.
+        ; with the last retry's verdict.
         Call RecoverFromHolders
     ${Else}
         ; Nothing is holding them. The three attempts in RetryDllPurge were two seconds

tools/installer-holder-probe/close_holders.cpp

@@ -1,5 +1,7 @@
 #include "close_holders.h"
 
+#include "restart_manager_holders.h"
+
 // clang-format off
 // windows.h has to come before anything that pulls in a Windows type, as everywhere in
 // this tool.
@@ -310,7 +312,7 @@ CloseHoldersResult closeCockatriceHolders(const std::vector<Holder> &holders, un
     return result;
 }
 
-EndHoldersResult endCockatriceHolders(const std::vector<Holder> &holders, unsigned long waitMilliseconds)
+EndHoldersResult endHolders(const std::vector<Holder> &holders, unsigned long waitMilliseconds)
 {
     EndHoldersResult result;
     std::vector<std::wstring> lines;
@@ -318,13 +320,16 @@ EndHoldersResult endCockatriceHolders(const std::vector<Holder> &holders, unsign
     std::vector<std::size_t> endedLines;
 
     for (const Holder &holder : holders) {
-        if (holder.isService || !isCockatriceImage(holder.imageName)) {
+        if (!isPayloadProgram(holder.imageName)) {
-            // The same split as the close: Oracle writes the card database in place and a
+            // Not one of the programs the payload installs: a scanner that has a DLL open, or
-            // service has nothing to ask, so neither is ever ended here.
+            // a program the user keeps in the install directory. It is named and left to the
+            // user, because ending it would destroy something this installer does not own.
             lines.push_back(processLabel(holder) + L" left to you");
             continue;
         }
 
+        // Services included: the installer asked before getting here, and the Service
+        // Control Manager reports the terminated process as a stopped service afterwards.
         const HANDLE process = OpenProcess(PROCESS_TERMINATE | SYNCHRONIZE, FALSE, holder.processId);
         if (process == nullptr) {
             lines.push_back(processLabel(holder) + L" could not be ended (win32 " + std::to_wstring(GetLastError()) +

tools/installer-holder-probe/close_holders.h

@@ -63,13 +63,20 @@ struct EndHoldersResult
 };
 
 /**
- * @brief Ends every running Cockatrice the probe named.
+ * @brief Ends every running Cockatrice, Oracle and Servatrice the probe named.
  *
  * This is the last resort behind the installer's "end it now" answer, never a first move: a
  * client that is still running may have an unsaved deck, an open game or a settings write in
- * flight, so the caller only gets here after the user picked it from a prompt that says so.
+ * flight, Oracle may be part-way through writing the card database, and Servatrice may be
- * Only cockatrice.exe is ended - Oracle writes the card database in place and Servatrice is a
+ * serving a game, so the caller only gets here after the user picked it from a prompt that
- * service, so both are named and left to the user, exactly as they are for the close.
+ * says so.
+ *
+ * Every program the payload installs is ended, services included. A service is terminated
+ * like a process rather than stopped through the Service Control Manager: the installer has
+ * asked the user first, and the SCM notices the process exit and marks the service stopped
+ * afterwards, so it can be started again normally. Anything else the Restart Manager named -
+ * a scanner holding a DLL, a program the user keeps in the install directory - is left to
+ * the user, because ending it would destroy something this installer does not own.
  *
  * Waits up to @c waitMilliseconds for what it ended to be gone, so the caller can measure the
  * purge immediately afterwards instead of racing a process that is still exiting.
@@ -77,6 +84,6 @@ struct EndHoldersResult
  * @param holders what the probe named, one entry per application or service
  * @param waitMilliseconds how long to wait for the ended instances to be gone, 0 to not wait
  */
-EndHoldersResult endCockatriceHolders(const std::vector<Holder> &holders, unsigned long waitMilliseconds);
+EndHoldersResult endHolders(const std::vector<Holder> &holders, unsigned long waitMilliseconds);
 
 #endif

tools/installer-holder-probe/holder_line.h

@@ -26,12 +26,15 @@ inline constexpr std::size_t HOLDER_LINE_MAX = 5;
 /**
  * @brief How many characters the summary may spend on the holders it names.
  *
- * The line is not the whole string: the abort dialog puts it after a fixed paragraph and
+ * The line is not the whole string: every prompt puts it after a fixed paragraph and before
- * before the path of the installer log, inside an NSIS string of 1024 characters. Windows
+ * the answers, and one NSIS string holds 1024 characters. Those paragraphs grew when the
- * caps one application or service name well below this budget, so it bounds the list
+ * guard started spelling out what ending the programs costs and what is left to do by hand,
- * without ever cutting a single holder short.
+ * so this budget is what keeps the longest message - the one that also says the last end
+ * freed nothing, plus the "; and N more" the summary appends after the budget - complete,
+ * including the Cancel line at its end. Windows caps one application or service name well
+ * below this, so it bounds the list without ever cutting a single holder short.
  */
-inline constexpr std::size_t HOLDER_LINE_CHAR_MAX = 400;
+inline constexpr std::size_t HOLDER_LINE_CHAR_MAX = 320;
 
 /**
  * @brief Renders one holder the way the user has to recognise it, e.g.

tools/installer-holder-probe/main.cpp

@@ -45,8 +45,9 @@ enum ExitCode
 ///                     abort dialog/log
 ///   --close-holders  bring the running Cockatrice forward, ask it to close, and wait for
 ///                     it to exit, for the offer to close after a failed purge
-///   --end-holders    end the running Cockatrice and wait for it to be gone, for the last
+///   --end-holders    end the running Cockatrice, Oracle and Servatrice and wait for them to
-///                     resort the guard prompt offers after a close was asked for twice
+///                     be gone, for the last resort the guard prompt offers when a close did
+///                     not free the files
 ///
 /// All three answer from two lookups that cover each other's blind spot: the Restart Manager
 /// names whoever has a payload file open, and every process running from the directory is
@@ -161,7 +162,7 @@ int main(int argumentCount, char **arguments)
         return EXIT_HOLDERS_FOUND;
     }
     if (mode == Mode::End) {
-        const EndHoldersResult result = endCockatriceHolders(holders, END_WAIT_MILLISECONDS);
+        const EndHoldersResult result = endHolders(holders, END_WAIT_MILLISECONDS);
         writeLine(result.report);
         return result.ended > 0 ? EXIT_HOLDERS_FOUND : EXIT_NO_HOLDERS;
     }

tools/installer-holder-probe/restart_manager_holders.cpp

@@ -34,16 +34,6 @@ constexpr const wchar_t *PAYLOAD_FILES[] = {L"cockatrice.exe", L"oracle.exe", L"
 /// exists to name, so askingProcessIds() stops its walk up the parent chain at one.
 constexpr const wchar_t *PAYLOAD_PROGRAMS[] = {L"cockatrice.exe", L"oracle.exe", L"servatrice.exe"};
 
-bool isPayloadProgram(const std::wstring &imageName)
-{
-    for (const wchar_t *program : PAYLOAD_PROGRAMS) {
-        if (CompareStringOrdinal(imageName.c_str(), -1, program, -1, TRUE) == CSTR_EQUAL) {
-            return true;
-        }
-    }
-    return false;
-}
-
 void collectFilesOf(const std::wstring &directory, int depth, int maxDepth, std::vector<std::wstring> &files)
 {
     if (files.size() >= MAX_PROBED_FILES) {
@@ -144,6 +134,16 @@ Holder holderOf(const RM_PROCESS_INFO &info)
 }
 } // namespace
 
+bool isPayloadProgram(const std::wstring &imageName)
+{
+    for (const wchar_t *program : PAYLOAD_PROGRAMS) {
+        if (CompareStringOrdinal(imageName.c_str(), -1, program, -1, TRUE) == CSTR_EQUAL) {
+            return true;
+        }
+    }
+    return false;
+}
+
 std::vector<std::wstring> collectPayloadFiles(const std::wstring &installDir)
 {
     std::vector<std::wstring> files;

tools/installer-holder-probe/restart_manager_holders.h

@@ -21,6 +21,18 @@
  */
 std::vector<std::wstring> collectPayloadFiles(const std::wstring &installDir);
 
+/**
+ * @brief Whether @p imageName is one of the programs the payload installs.
+ *
+ * The single place that answers which images are Cockatrice's own, so the guard ends the
+ * same set it names. Matched case-insensitively on the image name alone: only holders are
+ * ever tested against it, and a holder is a process that has a payload file open or runs
+ * from the install directory.
+ *
+ * @param imageName the file name of a process image, empty when it could not be read.
+ */
+bool isPayloadProgram(const std::wstring &imageName);
+
 /**
  * @brief Names the applications and services that have one of @p files open or mapped.
  *