diff --git a/cmake/NSIS.template.in b/cmake/NSIS.template.in index 72d5287fc..6b861a370 100644 --- a/cmake/NSIS.template.in +++ b/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 -; more for every "Yes" at the prompt. From the second one on, the prompt offers to end the -; instance instead (see GuardAppsNotRunning). +; Whether the guard has already asked Cockatrice to close by itself: 0 before the automatic +; close, 1 after it. The prompt that follows offers to end the instance rather than asking +; 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 -; Cockatrice may hold an unsaved deck and Oracle writes the card database in -; place. On /R the old version is about to be removed, so an answer the probe -; cannot give cancels the upgrade before it starts rather than after it. +; and the user is asked to close them; nothing is force-stopped without that +; answer, because Cockatrice may hold an unsaved deck and Oracle writes the card +; database in place. On /R the old version is about to be removed, so an answer +; 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 -; has no window to ask. A close is a request (WM_CLOSE), made automatically once -; because that is what the user started the installer to do, and again whenever -; they ask for it from the prompt. When it is not answered the user is told which -; program is in the way, and once a close has been asked for twice without freeing -; the files they are offered to end Cockatrice themselves - the last resort, with -; what it costs spelled out, and never applied to Oracle or Servatrice. +; the card database in place and can be killed mid-write, and Servatrice may be serving +; a game. A close is a request (WM_CLOSE), made automatically once because that is what +; the user started the installer to do, and only Cockatrice can answer one - the other +; two have no window. When the close is not answered the user is told which program is +; in the way and is offered to end all three themselves - the last resort, with what it +; costs spelled out in the prompt. + +; 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 -; answer the prompt offers once a close has been asked for twice and ignored, spelled out -; with what it costs - see EndHolders - and it is only ever applied to cockatrice.exe. +; Never a force-close here: the application decides whether to quit. Ending is a separate +; answer the guard prompt offers when a close did not free the files, spelled out with what +; 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 -; ends cockatrice.exe and nothing else - Oracle writes the card database in place and -; Servatrice is a service, so both stay the user's job, exactly as they do for the close. +; still running may have an unsaved deck, a game or a settings write in flight, Oracle may +; be part-way through writing the card database, and Servatrice may be serving a game. The +; 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 -; user closed them by hand (No), and Cancel. Once a close has been asked for twice without -; freeing the files, it offers to end Cockatrice instead - the last resort, see EndHolders. +; its own confirmation. It offers to end the programs the payload installs - and only those, +; which the text says - with what that costs (Yes), a plain re-check for after the user +; 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 - ; stops being a plan: the instance is either waiting on a question the user has not - ; answered, or stuck where a close cannot reach it. Offer to end it - the only thing - ; left that can free the files - with the cost in the text, because a client that is - ; still running may have an unsaved deck, a game or a settings write in flight. - ${If} $GuardCloseTried >= 2 - MessageBox MB_YESNO|MB_ICONEXCLAMATION|MB_DEFBUTTON2 \ - "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" \ - IDYES guard_end IDNO guard_close - !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_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 + ; The automatic close already ran once. Asking Cockatrice to close again is unlikely + ; to help: an instance that did not answer the first one is usually sitting on a + ; confirmation of its own, and the close probe leaves such an instance alone so that a + ; second WM_CLOSE cannot hide the window the question is in - which is how a pre-3.1.2 + ; update used to leave a headless process behind. An instance with no window has + ; nothing to ask at all. Ending is what is left that can free the files, so the prompt + ; offers it here, with what it costs in the text: an unsaved deck, a game, a settings + ; write, and an Oracle that may be mid-way through the card database. The answers that + ; end nothing stay reachable from the same dialog: the instance's own confirmation, + ; and closing anything by hand. + !insertmacro CockEndPrompt "Cockatrice, Oracle or Servatrice is still using files of the installation:" $GuardOutput "installation" + MessageBox MB_YESNOCANCEL|MB_ICONEXCLAMATION|MB_DEFBUTTON2 "$GuardPrompt" IDYES guard_end 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: - IntOp $GuardCloseTried $GuardCloseTried + 1 - !insertmacro CockLog "guard: closing cockatrice on request, then checking again" - Call CloseHolders + guard_end: + !insertmacro CockLog "guard: ending the payload programs on request, then checking again" + Call EndHolders + ; 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 - ; image and its Qt DLLs mapped, so writing the payload over them is the broken - ; install this wait exists to prevent - and the guard that runs next names the - ; same process by its image path and stops there anyway, so an "ignore" would - ; only put the same question a moment later, after a close attempt nobody asked - ; for. Waiting again is what helps: the client is most likely showing its own - ; "quit anyway?" confirmation, and that window is the thing to answer. - 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. 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." \ - IDRETRY wait_launcher_retry + ; Same three answers as every other prompt that waits on a running program, and + ; "carry on anyway" is not one of them: a live process has its image and its Qt + ; DLLs mapped, so writing the payload over them is the broken install this wait + ; exists to prevent. Yes ends the launcher, No asks it to close again and keeps + ; waiting, Cancel cancels - the launcher is the client the user started this update + ; from, and ending it is the same offer the guard makes a moment later. + MessageBox MB_YESNOCANCEL|MB_ICONEXCLAMATION|MB_DEFBUTTON2 \ + "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." \ + IDYES wait_launcher_end IDNO 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 -; again, and retry while the user keeps asking. Sets $StaleRuntimeAfterPurge to the last -; purge's verdict, so the caller still aborts when the files survive. +; the old version is already uninstalled - it closes once, purges again, and only then +; asks: end what is still holding the files, or try the removal again by hand. Sets +; $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 -; button) and keeps the last word: it may be waiting for an answer to its own "really -; quit?" confirmation (a running card database 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. Oracle and Servatrice have no -; 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 -; named that this installer cannot close - those two included - is left to the user, whom -; the prompt names it to. +; The shape is the guard's, deliberately: one automatic attempt at a graceful close, then a +; prompt whose Yes ends the payload programs and whose No retries. A client that was still +; shutting down releases the files in the automatic round and never sees a question, and an +; Oracle or Servatrice - which a close cannot reach - is what the prompt exists for, so the +; user is never left with a dialog whose answers cannot free anything. +; +; The automatic close is a request (WM_CLOSE) and the client keeps the last word: it may be +; waiting for an answer to its own "really quit?" confirmation (a running card database +; 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 -; again without asking it to close (for when the user closed things by hand), and Cancel -; gives up. The close stays on offer after one that did nothing because the usual reason is -; Cockatrice's own confirmation still waiting for an answer - the probe waits for that, the -; user can answer it in the application's window, and the next round can then collect the -; 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. +; Every round offers all three answers: Yes ends the programs the payload installs - and +; says so, because anything else the probe named is left to the user - and purges again, No +; purges again without ending anything (for when the user closed things by hand), and +; Cancel gives up. 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: - MessageBox MB_YESNOCANCEL|MB_ICONEXCLAMATION|MB_DEFBUTTON1 \ - "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." \ - 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 $GuardEndFailed 0 + ; The automatic attempt, the same one the guard makes before it asks anything. + !insertmacro CockLog "recover-holders: closing cockatrice, then trying the removal again" 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 - ; nothing, offer to end the instance, with what it costs in the text. - ${If} $GuardCloseTried >= 2 - MessageBox MB_YESNO|MB_ICONEXCLAMATION|MB_DEFBUTTON2 \ - "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" \ - IDYES un_guard_end IDNO un_guard_close - !insertmacro CockLogUn "uninst-guard: cancelled by user" - SetErrorLevel 1 - Abort - ; 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 + ; Same approach as in the installer's guard: the automatic close already ran once, and + ; asking again is unlikely to help an instance that is sitting on its own confirmation + ; or has no window a close could reach. Ending is what is left that can free the files, + ; so the prompt offers it here, with what it costs in the text, while keeping the + ; answers that end nothing - the instance's own question, and closing anything by hand + ; - reachable from the same dialog. CockEndPrompt words it, so this copy and the + ; installer's promise the same thing. + !insertmacro CockEndPrompt "Cockatrice, Oracle or Servatrice is still using files of the installation:" $GuardOutput "uninstallation" + MessageBox MB_YESNOCANCEL|MB_ICONEXCLAMATION|MB_DEFBUTTON2 "$GuardPrompt" IDYES un_guard_end 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: - IntOp $GuardCloseTried $GuardCloseTried + 1 - !insertmacro CockLogUn "uninst-guard: closing cockatrice on request, then checking again" - Call un.CloseHolders + un_guard_end: + !insertmacro CockLogUn "uninst-guard: ending the payload programs on request, then checking again" + Call un.EndHolders + ; 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, - ; repeating while the user keeps saying Yes; this updates - ; $StaleRuntimeAfterPurge with the last retry's verdict. + ; leaves the user with no working Cockatrice. It closes once, then offers to end + ; what is left or to try the removal again; this updates $StaleRuntimeAfterPurge + ; with the last retry's verdict. Call RecoverFromHolders ${Else} ; Nothing is holding them. The three attempts in RetryDllPurge were two seconds diff --git a/tools/installer-holder-probe/close_holders.cpp b/tools/installer-holder-probe/close_holders.cpp index d5bd5251f..791347408 100644 --- a/tools/installer-holder-probe/close_holders.cpp +++ b/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 &holders, un return result; } -EndHoldersResult endCockatriceHolders(const std::vector &holders, unsigned long waitMilliseconds) +EndHoldersResult endHolders(const std::vector &holders, unsigned long waitMilliseconds) { EndHoldersResult result; std::vector lines; @@ -318,13 +320,16 @@ EndHoldersResult endCockatriceHolders(const std::vector &holders, unsign std::vector endedLines; for (const Holder &holder : holders) { - if (holder.isService || !isCockatriceImage(holder.imageName)) { - // The same split as the close: Oracle writes the card database in place and a - // service has nothing to ask, so neither is ever ended here. + if (!isPayloadProgram(holder.imageName)) { + // Not one of the programs the payload installs: a scanner that has a DLL open, or + // 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()) + diff --git a/tools/installer-holder-probe/close_holders.h b/tools/installer-holder-probe/close_holders.h index 4f9f5f0af..0e2b93b26 100644 --- a/tools/installer-holder-probe/close_holders.h +++ b/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. - * Only cockatrice.exe is ended - Oracle writes the card database in place and Servatrice is a - * service, so both are named and left to the user, exactly as they are for the close. + * flight, Oracle may be part-way through writing the card database, and Servatrice may be + * serving a game, so the caller only gets here after the user picked it from a prompt that + * 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 &holders, unsigned long waitMilliseconds); +EndHoldersResult endHolders(const std::vector &holders, unsigned long waitMilliseconds); #endif diff --git a/tools/installer-holder-probe/holder_line.h b/tools/installer-holder-probe/holder_line.h index 7fcc92375..163112ff6 100644 --- a/tools/installer-holder-probe/holder_line.h +++ b/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 - * before the path of the installer log, inside an NSIS string of 1024 characters. Windows - * caps one application or service name well below this budget, so it bounds the list - * without ever cutting a single holder short. + * The line is not the whole string: every prompt puts it after a fixed paragraph and before + * the answers, and one NSIS string holds 1024 characters. Those paragraphs grew when the + * guard started spelling out what ending the programs costs and what is left to do by hand, + * 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. diff --git a/tools/installer-holder-probe/main.cpp b/tools/installer-holder-probe/main.cpp index 3f1556b75..0cbecb9f2 100644 --- a/tools/installer-holder-probe/main.cpp +++ b/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 -/// resort the guard prompt offers after a close was asked for twice +/// --end-holders end the running Cockatrice, Oracle and Servatrice and wait for them to +/// 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; } diff --git a/tools/installer-holder-probe/restart_manager_holders.cpp b/tools/installer-holder-probe/restart_manager_holders.cpp index 1abfcb352..e8c982f26 100644 --- a/tools/installer-holder-probe/restart_manager_holders.cpp +++ b/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 &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 collectPayloadFiles(const std::wstring &installDir) { std::vector files; diff --git a/tools/installer-holder-probe/restart_manager_holders.h b/tools/installer-holder-probe/restart_manager_holders.h index 044cbe449..c14499941 100644 --- a/tools/installer-holder-probe/restart_manager_holders.h +++ b/tools/installer-holder-probe/restart_manager_holders.h @@ -21,6 +21,18 @@ */ std::vector 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. *