diff --git a/cmake/NSIS.template.in b/cmake/NSIS.template.in index ceba04877..8d7a82f81 100644 --- a/cmake/NSIS.template.in +++ b/cmake/NSIS.template.in @@ -61,12 +61,8 @@ Var GuardOutput ; 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. +; The question CockEndPrompt assembles: fixed sentences around the probe's line, and the +; result has to stay inside the characters one NSIS string holds. Var GuardPrompt Var WaitPid Var LauncherExited @@ -1043,8 +1039,9 @@ FunctionEnd ; 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. +; What the user does instead when they say no: the prompt's other answer stops the run, so they +; are told how to get past the same wall next time. Also used by the abort dialogs, which is +; where the answer to "why did it stop" ends up. !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 @@ -1056,23 +1053,22 @@ FunctionEnd ; 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. ; +; Two answers, one of each: the automatic close already ran when this is asked, so Yes is the +; only thing left that can free the files and No stops the run. There is deliberately no +; "try again" - the same question, word for word, with nothing changed in between was what +; made an abort out of a stall. +; ; 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. +; longest listing that can arrive 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" +; @param cancelWord what No 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}." + 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 cancel the ${cancelWord}. ${AFTER_CLOSING_GUIDANCE}" !macroend ; Emitted once for the installer and once for the uninstaller: the uninstaller @@ -1115,10 +1111,12 @@ FunctionEnd ; cockatrice.exe is asked: Oracle writes the card database in place and Servatrice has no ; 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 -; update is running, a game is open or a deck is unsaved - and that question is only useful -; if it is in front of the installer instead of behind it. +; Each instance's own window is asked to close with WM_CLOSE - whether it is on screen or +; only in the notification area, since the request does not need a visible window and the +; notification-area instance is the one this update has no other way to reach. A visible +; window is brought forward first. WM_CLOSE is answered, not obeyed - Cockatrice asks before +; it quits while a card database update is running, a game is open or a deck is unsaved - and +; that question is only useful if it is in front of the installer instead of behind it. ; ; The probe waits up to 30 s for the processes it asked to exit, so the check that follows ; measures a settled directory rather than sleeping for a fixed time. Its line goes into the @@ -1171,14 +1169,16 @@ FunctionEnd ; usage: set $GuardDir to the directory to guard, then call this. ; -; Asks Cockatrice to close once, then prompts until the payload is free. The automatic -; close is the same request clicking the window's close button makes, so the common case - -; 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 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. +; Asks Cockatrice to close once, then asks one question with one answer that can free the +; payload. The automatic close is the same request clicking the window's close button makes, +; so the common case - 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 a client that is sitting on its own +; confirmation needs an answer nobody can give it from here. It offers to end the programs +; the payload installs - and only those, which the text says - with what that costs (Yes), +; and its other answer stops the install (No) instead of repeating the same dialog. What +; survives an end is reported in the abort, never asked about again - 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 @@ -1188,7 +1188,6 @@ FunctionEnd Function GuardAppsNotRunning !insertmacro CockLog "guard: checking $GuardDir (launcherExited=$LauncherExited)" StrCpy $GuardCloseTried 0 - StrCpy $GuardEndFailed 0 guard_check: Call RunHolderProbe ${If} $GuardExit == "0" @@ -1211,21 +1210,15 @@ Function GuardAppsNotRunning Goto guard_check ${EndIf} DetailPrint "The install directory is in use by: $GuardOutput" - ; 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. + ; The close ran and did not free the files, so ending them is the only thing left that + ; can - and that is the one question, asked once. Its other answer stops the run + ; instead of asking again: a second identical dialog, with nothing changed in between, + ; is what made users cancel an update the installer could have finished by itself. !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 + MessageBox MB_YESNO|MB_ICONEXCLAMATION|MB_DEFBUTTON2 "$GuardPrompt" IDYES guard_end !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." + "The installation was cancelled because Cockatrice, Oracle or Servatrice is still running and holding files of the previous version.$\r$\n$\r$\n${AFTER_CLOSING_GUIDANCE}$\r$\n$\r$\nClose them and run the installer again." ; RemoveOldPayloadLeftovers runs with the pre-uninstall backup already taken; ; restoring here keeps this abort from stranding the only copy in %TEMP%. Call RestoreUserDataAfterUninstall @@ -1234,12 +1227,25 @@ Function GuardAppsNotRunning 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 + ; Measure once and report, rather than asking the same question again. Everything the + ; payload installs was ended, so what survived is a program this installer may not + ; open or does not own - another dialog offering to end it again would only repeat + ; itself. + Call RunHolderProbe + ${If} $GuardExit == "0" + !insertmacro CockLog "guard: nothing is holding the payload" + Return + ${EndIf} + !insertmacro CockLog "guard: still in use after ending them ($GuardExit): $GuardOutput" + ${If} $GuardExit == "1" + StrCpy $AbortReason \ + "The installation was cancelled because these programs still hold files of the previous version and could not be ended:$\r$\n$\r$\n$GuardOutput$\r$\n$\r$\n${AFTER_CLOSING_GUIDANCE}$\r$\n$\r$\nClose them and run the installer again." + ${Else} + StrCpy $AbortReason \ + "The installation was cancelled because, after ending Cockatrice, Oracle and Servatrice, the installer could not check whether they still hold files of the previous version.$\r$\n$\r$\nThe check reported: $GuardOutput$\r$\n$\r$\nClose them and run the installer again." + ${EndIf} + Call RestoreUserDataAfterUninstall + Call AbortInstall ${EndIf} ; Not an answer at all: the probe was not configured, the Restart Manager ; call failed and nothing runs from the directory, or it timed out. Carrying @@ -1921,36 +1927,20 @@ FunctionEnd ; a decision, it asks for one. No offer when the probe named nobody: there is nothing to ; end, and the abort's per-file diagnostics already record what Windows said. ; -; 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. +; One question after the automatic close, and never a loop: Yes ends the programs the +; payload installs - and says so, because anything else the probe named is left to the user - +; and purges again; the other answer gives up, and the caller's abort names what survived. +; The listing is re-read before the question, so it says what is left now rather than +; repeating the first one. Function RecoverFromHolders ${If} $HolderFound != 1 !insertmacro CockLog "recover-holders: no holder was named, nothing to offer" Return ${EndIf} - 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 !insertmacro CockLogListing "after-retry-purge" "*.dll" @@ -1960,13 +1950,33 @@ Function RecoverFromHolders DetailPrint "Files of the previous version were removed; continuing the installation." Return ${EndIf} - ; Still stuck: refresh the names, so the next prompt and the log describe what - ; survived the last attempt and not what was there before it. + ; Still stuck: refresh the names, so the prompt and the log describe what survived the + ; last attempt and not what was there before it. Call LogStaleRuntimeDiagnostics ${If} $HolderFound != 1 Return ${EndIf} - Goto rfh_prompt + ; One question, the same one the guard asks and with the same two answers. No retry loop: + ; the purge has already been tried after the close, so the only thing that can still free + ; the files is ending them, and the caller's abort names whatever survives that. + !insertmacro CockEndPrompt "Files of the previous version of Cockatrice are still in use by:" $HolderOutput "installation" + MessageBox MB_YESNO|MB_ICONEXCLAMATION|MB_DEFBUTTON2 "$GuardPrompt" IDYES rfh_end + !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 + Call RetryDllPurge + StrCpy $StaleRuntimeAfterPurge $R0 + !insertmacro CockLog "recover-holders: stale-runtime after end=$StaleRuntimeAfterPurge" + ${If} $StaleRuntimeAfterPurge = 0 + DetailPrint "Files of the previous version were removed; continuing the installation." + Return + ${EndIf} + ; Refresh the names one last time so the abort that follows describes what survived the + ; end, and then let it stop the run: the same dialog again would only repeat itself. + Call LogStaleRuntimeDiagnostics FunctionEnd ; Uninstaller copy of the guard (the uninstaller gets its own function set compiled in, it @@ -1980,7 +1990,6 @@ FunctionEnd Function un.GuardAppsNotRunning StrCpy $GuardCloseTried 0 - StrCpy $GuardEndFailed 0 un_guard_check: !insertmacro CockLogUn "uninst-guard: checking $GuardDir" Call un.RunHolderProbe @@ -1999,15 +2008,12 @@ Function un.GuardAppsNotRunning Goto un_guard_check ${EndIf} DetailPrint "The install directory is in use by: $GuardOutput" - ; 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 + ; Same question as in the installer's guard, and the same two answers: Yes ends the + ; programs the payload installs, anything else stops the uninstallation. Asking again + ; after a No only repeated the 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 + MessageBox MB_YESNO|MB_ICONEXCLAMATION|MB_DEFBUTTON2 "$GuardPrompt" IDYES un_guard_end !insertmacro CockLogUn "uninst-guard: cancelled by user" SetErrorLevel 1 Abort @@ -2015,12 +2021,17 @@ Function un.GuardAppsNotRunning 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 + ; One measurement and a stop, not another question: what survived a terminate is a + ; program this uninstaller may not open or does not own, and ending it again cannot + ; change that. + Call un.RunHolderProbe + ${If} $GuardExit == "0" + !insertmacro CockLogUn "uninst-guard: nothing is holding the payload" + Return + ${EndIf} + !insertmacro CockLogUn "uninst-guard: still in use after ending them ($GuardExit): $GuardOutput" + SetErrorLevel 1 + Abort ${EndIf} ; Same reasoning as in the installer: without an answer from the probe we cannot tell ; whether a running instance still holds file locks, and refusing outright would leave @@ -2333,8 +2344,8 @@ ${If} $StaleRuntimeAfterPurge = 1 ${If} $HolderFound = 1 ; On /R the previous version is already uninstalled at this point, so an abort ; 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. + ; what is left; this updates $StaleRuntimeAfterPurge with the verdict after that + ; close, or after the end when the user asked for one. Call RecoverFromHolders ${Else} ; Nothing is holding them. The three attempts in RetryDllPurge were two seconds diff --git a/cmake/SendInstallerLog.ps1.in b/cmake/SendInstallerLog.ps1.in index dfb8c3d63..585c726dd 100644 --- a/cmake/SendInstallerLog.ps1.in +++ b/cmake/SendInstallerLog.ps1.in @@ -57,6 +57,29 @@ try { } } + # --- What was holding the files, read from the log as well ------------------- + # The guard and the close attempt each log one line naming every holder and what + # happened to it. Carrying the last of each as event data keeps an abort caused by + # a running instance actionable without opening the attachment. Kept out of the + # message so the issue still groups on the reason alone. + $Holders = '' + $CloseResult = '' + $EndResult = '' + if (Test-Path -LiteralPath $LogPath) { + $holderLine = Select-String -LiteralPath $LogPath -Pattern 'guard: still in use by (.*)$' | Select-Object -Last 1 + if ($holderLine) { + $Holders = $holderLine.Matches[0].Groups[1].Value + } + $closeLine = Select-String -LiteralPath $LogPath -Pattern '(close-holders: .*)$' | Select-Object -Last 1 + if ($closeLine) { + $CloseResult = $closeLine.Matches[0].Groups[1].Value + } + $endLine = Select-String -LiteralPath $LogPath -Pattern '(end-holders: .*)$' | Select-Object -Last 1 + if ($endLine) { + $EndResult = $endLine.Matches[0].Groups[1].Value + } + } + # --- Event --------------------------------------------------------------- $eventId = [guid]::NewGuid().ToString('N') $now = (Get-Date).ToUniversalTime().ToString('yyyy-MM-ddTHH:mm:ssZ') @@ -75,6 +98,9 @@ try { extra = @{ reason = $Reason os = [System.Environment]::OSVersion.VersionString + holders = $Holders + close_result = $CloseResult + end_result = $EndResult } } $eventJson = $sentryEvent | ConvertTo-Json -Compress -Depth 6 diff --git a/cockatrice/src/interface/widgets/visual_deck_storage/deck_preview/deck_preview_deck_tags_display_widget.cpp b/cockatrice/src/interface/widgets/visual_deck_storage/deck_preview/deck_preview_deck_tags_display_widget.cpp index 1a06c0b18..11c8df54a 100644 --- a/cockatrice/src/interface/widgets/visual_deck_storage/deck_preview/deck_preview_deck_tags_display_widget.cpp +++ b/cockatrice/src/interface/widgets/visual_deck_storage/deck_preview/deck_preview_deck_tags_display_widget.cpp @@ -12,9 +12,9 @@ #include #include -#include #include #include +#include #include #include #include diff --git a/tools/installer-holder-probe/close_holders.cpp b/tools/installer-holder-probe/close_holders.cpp index d1831e4e9..53a1fed8e 100644 --- a/tools/installer-holder-probe/close_holders.cpp +++ b/tools/installer-holder-probe/close_holders.cpp @@ -103,44 +103,50 @@ bool isAnswerableWindow(HWND window) /// Picks the window to post the close to and the one the user should end up looking at. struct CloseTarget { - HWND main = nullptr; ///< first visible window without an owner: the application's own + HWND main = nullptr; ///< first answerable window without an owner: the application's own HWND toRaise = nullptr; ///< topmost visible window: a dialog when one is up - bool modalOpen = false; ///< the only window a user could answer is the instance's own question + bool hidden = false; ///< the window being closed is not on screen (notification area) }; -/// Only the windows the user could answer are candidates - see isAnswerableWindow. An -/// instance left with nothing but Qt's own bookkeeping windows visible has no window to ask: -/// the caller reports that instead of posting a close the application would answer with a -/// terminate that skips its own confirmation. -CloseTarget chooseCloseTarget(const std::vector &windows) +/// Only the windows the user could be asked to answer are candidates - see isAnswerableWindow. +/// +/// A hidden window is a candidate like a visible one: WM_CLOSE does not need a window on +/// screen, so an instance that is only in the notification area answers it the way it answers +/// its own close button, and the confirmation it raises then is a window Qt shows itself - +/// painted, unlike one restored from outside. Nothing is shown from here. Qt's own bookkeeping +/// windows stay excluded, because a close posted to the notification-area one terminates the +/// instance without the confirmation and that is worse than no close at all. +/// +/// @param windows every top-level window the process owns, in Z-order +/// @param askHidden when false, only a window that is on screen is a target: the launcher +/// mode reads "no window" as "past its own prompts and stuck", which stops being true +/// the moment a hidden window is asked to close +CloseTarget chooseCloseTarget(const std::vector &windows, bool askHidden) { CloseTarget target; - HWND topmost = nullptr; for (const HWND window : windows) { - if (IsWindowVisible(window) == FALSE || !isAnswerableWindow(window)) { + if (!isAnswerableWindow(window)) { continue; } - if (topmost == nullptr) { - topmost = window; + const bool visible = IsWindowVisible(window) != FALSE; + if (visible && target.toRaise == nullptr) { + target.toRaise = window; } - if (target.main == nullptr && GetWindow(window, GW_OWNER) == nullptr) { + if (target.main == nullptr && GetWindow(window, GW_OWNER) == nullptr && (visible || askHidden)) { target.main = window; + target.hidden = !visible; } } - target.toRaise = topmost != nullptr ? topmost : target.main; - // A modal dialog disables the windows behind it, so a disabled main window is the - // instance asking a question of its own. A visible window with no unowned main window - // next to it is one too: what is on screen is then the question itself - the window - // behind it is hidden, not gone - and there is no window a close could be posted to - // without hiding the question with it. - target.modalOpen = target.toRaise != nullptr && (target.main == nullptr || IsWindowEnabled(target.main) == FALSE); return target; } /// What is left of the window the close was posted to, so a log line tells an instance that /// answered by hiding from one that never answered at all. A process that hid its window and -/// stayed alive is the one that needs the next close to bring it back into view. -std::wstring describeWindowState(HWND window) +/// stayed alive is the one the guard's end answer is for. +/// +/// @param window the window the close was posted to, or null when there was none +/// @param wasHidden whether that window was already off screen before the close +std::wstring describeWindowState(HWND window, bool wasHidden) { if (window == nullptr || IsWindow(window) == FALSE) { return L", its window is gone"; @@ -151,7 +157,12 @@ std::wstring describeWindowState(HWND window) DWORD_PTR ignored = 0; const bool responding = SendMessageTimeoutW(window, WM_NULL, 0, 0, SMTO_ABORTIFHUNG | SMTO_BLOCK, 1000, &ignored) != 0; - std::wstring state = IsWindowVisible(window) != FALSE ? L", window still open" : L", window hidden again"; + std::wstring state; + if (IsWindowVisible(window) != FALSE) { + state = L", window still open"; + } else { + state = wasHidden ? L", window still hidden" : L", window hidden again"; + } if (!responding) { state += L" and not responding"; } @@ -214,13 +225,15 @@ bool isCockatriceImage(const std::wstring &imageName) return CompareStringOrdinal(imageName.c_str(), -1, COCKATRICE_IMAGE, -1, TRUE) == CSTR_EQUAL; } -CloseHoldersResult closeCockatriceHolders(const std::vector &holders, unsigned long waitMilliseconds) +CloseHoldersResult +closeCockatriceHolders(const std::vector &holders, unsigned long waitMilliseconds, bool askHiddenWindows) { CloseHoldersResult result; std::vector lines; std::vector asked; std::vector askedLines; std::vector askedWindows; + std::vector askedWasHidden; for (const Holder &holder : holders) { if (holder.isService || !isCockatriceImage(holder.imageName)) { @@ -233,38 +246,36 @@ CloseHoldersResult closeCockatriceHolders(const std::vector &holders, un WindowSearch search; search.processId = holder.processId; EnumWindows(collectProcessWindow, reinterpret_cast(&search)); - if (search.windows.empty()) { + CloseTarget target = chooseCloseTarget(search.windows, askHiddenWindows); + if (target.main == nullptr) { + // Nothing of the application's own to post a close to: every window left is one of + // Qt's bookkeeping windows, or the only thing on screen is a question whose owner + // window is gone. Neither is a close the application would answer - a close to the + // notification-area window terminates without the confirmation - so the instance is + // named and the guard's end answer is what can free its files. ++result.withoutWindow; - lines.push_back(processLabel(holder) + L" has no window to close, left to you"); + lines.push_back(processLabel(holder) + L" has no window of its own to close"); continue; } - const CloseTarget target = chooseCloseTarget(search.windows); - if (target.toRaise == nullptr) { - // Nothing visible that a user could answer: the instance is hidden - in the - // notification area, say - or has nothing but Qt's own bookkeeping windows on - // screen. Neither is a window a close could reach, and showing one from here does - // not help: Windows would display it while the application still believes it is - // hidden, so it stays blank and answers nothing. The instance is named and left to - // the user, who can open it from the notification area. - ++result.withoutWindow; - lines.push_back(processLabel(holder) + L" has no window to ask, left to you"); - continue; - } std::wstring line = processLabel(holder); - line += bringForward(target.toRaise) ? L" brought forward" : L" could not be brought forward"; - - // Posting the close to an instance that is asking a question of its own is what made - // an earlier attempt useless: the client accepts it, hides this window and the dialog - // with it, and stays alive inside the nested loop the dialog opened, where nothing can - // reach it any more. The question is the request now, so it is brought forward and - // left alone - including a question whose owner window is hidden, which is the only - // thing left on screen and has no unowned window a close could be posted to. - if (target.modalOpen) { - lines.push_back(line + L", and is waiting for an answer to a question of its own"); - continue; + if (target.hidden) { + line += L" is hidden in the notification area"; + } else { + if (target.toRaise == nullptr) { + target.toRaise = target.main; + } + line += bringForward(target.toRaise) ? L" brought forward" : L" could not be brought forward"; } + // Posted to the instance's own window even when a question of its own is up. The close + // is what that window's close button does, and the application is the one that answers + // it: a current client refuses the close while its confirmation is pending, leaving the + // question in front, and only a client from before that guard accepts it and hides the + // window with the question. Withholding the request therefore protected nobody - it + // left the installer asking the user to close a program it had never asked - while the + // guard's end answer is what frees the files when the close cannot. + // // Kept open across the wait so a pid reused after the process exits cannot be // mistaken for the instance that was asked to close. const HANDLE process = OpenProcess(SYNCHRONIZE, FALSE, holder.processId); @@ -287,6 +298,7 @@ CloseHoldersResult closeCockatriceHolders(const std::vector &holders, un askedLines.push_back(lines.size()); asked.push_back(process); askedWindows.push_back(target.main); + askedWasHidden.push_back(target.hidden); } lines.push_back(std::move(line)); } @@ -300,7 +312,7 @@ CloseHoldersResult closeCockatriceHolders(const std::vector &holders, un lines[askedLines[index]] += L", exited after " + std::to_wstring(elapsed) + L" ms"; } else { lines[askedLines[index]] += L", still running after " + std::to_wstring(elapsed) + L" ms" + - describeWindowState(askedWindows[index]); + describeWindowState(askedWindows[index], askedWasHidden[index]); } } } diff --git a/tools/installer-holder-probe/close_holders.h b/tools/installer-holder-probe/close_holders.h index 643624779..91e6b316b 100644 --- a/tools/installer-holder-probe/close_holders.h +++ b/tools/installer-holder-probe/close_holders.h @@ -10,15 +10,15 @@ * @brief What asking the holders to close did, in the form the installer logs. * * @c report is one line naming every holder, including the ones this deliberately - * leaves alone, because the installer repeats it in its log and in the second prompt. - * @c withoutWindow counts the holders that had no visible window a user could answer: they - * were named and left alone, and the launcher mode reads that count as "nothing to ask" - * rather than as a window the user still has to answer. + * leaves alone, because the installer repeats it in its log. + * @c withoutWindow counts the holders that had no window of their own to ask: they were + * named and left alone, and the launcher mode reads that count as "nothing to ask" rather + * than as a window the user still has to answer. */ struct CloseHoldersResult { unsigned int askedToClose = 0; ///< holders a WM_CLOSE was posted to - unsigned int withoutWindow = 0; ///< holders with no visible window a user could answer + unsigned int withoutWindow = 0; ///< holders with no window of their own to ask std::wstring report; ///< one line, narrowed to ASCII when written }; @@ -42,27 +42,34 @@ bool isCockatriceImage(const std::wstring &imageName); * asked to close. Oracle and Servatrice are left to the user - Oracle writes the card * database in place - and so is every service. * - * One thing happens per instance: its main window is brought to the foreground, because - * Cockatrice may answer the close with a confirmation of its own that would otherwise open - * behind the installer's prompt. Then WM_CLOSE is posted to the main window, the same - * request as clicking its close button - unless the instance is asking a question of its - * own, which disables that window: posting the close then only hides the question, so it is - * brought forward and left to the user. An instance with no window a user could answer - - * one hidden in the notification area, say - is named and left to the user as well; showing - * its windows from here does not help, because the application would still believe they are - * hidden and never paint them. Stopping the process is never attempted. + * One thing happens per instance: its own window is asked to close with WM_CLOSE, the same + * request as clicking that window's close button - whether it is on screen or only in the + * notification area, since the request does not need the window to be visible and the + * confirmation Cockatrice answers it with is a window Qt shows itself, painted, unlike one + * restored from outside. A window that is on screen is brought forward first, because that + * confirmation would otherwise open behind the installer's prompt; nothing is shown from + * here. Qt's own bookkeeping windows are never the target - posting a close to the + * notification-area one terminates the instance without its confirmation - so a process + * without a window of its own is named and left to the guard, which offers to end it. + * Stopping the process is never attempted here. * * Once the requests are out, this waits up to @c waitMilliseconds for the processes to * exit, so the caller can measure the purge immediately afterwards instead of sleeping * for a fixed time or re-asking a question this call already answered. A process that is - * still running is described further: whether its window is gone, still open, hidden - * again, and whether it answers at all, so the log can tell an instance that refused the - * close from one that never received it. + * still running is described further: whether its window is gone, still open, still + * hidden or hidden again, and whether it answers at all, so the log can tell an instance + * that refused the close from one that never received it. * * @param holders the Restart Manager answer, one entry per application or service * @param waitMilliseconds how long to wait for the asked instances to exit, 0 to not wait + * @param askHiddenWindows whether a window that is not on screen is asked to close too. + * The guard wants that: an instance in the notification area is the one this update + * has no other way to reach. The launcher mode does not - a launcher without a + * visible window has already answered its own close and is stuck in shutdown, and + * the installer ends it instead of putting a question to a user who is not there. */ -CloseHoldersResult closeCockatriceHolders(const std::vector &holders, unsigned long waitMilliseconds); +CloseHoldersResult +closeCockatriceHolders(const std::vector &holders, unsigned long waitMilliseconds, bool askHiddenWindows); /** * @brief What ending the holders did, in the form the installer logs. diff --git a/tools/installer-holder-probe/holder_line.h b/tools/installer-holder-probe/holder_line.h index 163112ff6..675d79fc6 100644 --- a/tools/installer-holder-probe/holder_line.h +++ b/tools/installer-holder-probe/holder_line.h @@ -29,9 +29,9 @@ inline constexpr std::size_t HOLDER_LINE_MAX = 5; * 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 + * so this budget is what keeps the longest message - the one with the longest listing, plus + * the "; and N more" the summary appends after the budget - complete, including the sentence + * that offers the other answer 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 = 320; diff --git a/tools/installer-holder-probe/main.cpp b/tools/installer-holder-probe/main.cpp index 10a6c22e4..d88760b35 100644 --- a/tools/installer-holder-probe/main.cpp +++ b/tools/installer-holder-probe/main.cpp @@ -148,9 +148,9 @@ bool launcherProcessIdFromEnvironment(unsigned long &processId) /// /// The guard closes a set and reports every holder; here there is exactly one process, and /// the installer needs an answer it can act on rather than a sentence: a launcher with no -/// window to ask is past its own prompts and stuck in shutdown, so no answer the installer -/// offers would ever reach it. Everything else - a window that was asked, or one that is -/// waiting on a question of its own - is still the user's decision. +/// visible window has already answered its own close and is stuck in shutdown, so no answer +/// the installer offers would ever reach it. A window that was asked - including one the +/// instance is showing a question in - is still the user's decision. int closeLauncher(const std::wstring &installDir) { unsigned long processId = 0; @@ -170,7 +170,7 @@ int closeLauncher(const std::wstring &installDir) writeLine(L"the launcher is not cockatrice.exe, left to you"); return EXIT_PROBE_FAILED; } - const CloseHoldersResult result = closeCockatriceHolders({holder}, CLOSE_WAIT_MILLISECONDS); + const CloseHoldersResult result = closeCockatriceHolders({holder}, CLOSE_WAIT_MILLISECONDS, false); writeLine(result.report); if (result.askedToClose > 0) { return EXIT_HOLDERS_FOUND; @@ -251,7 +251,9 @@ int main(int argumentCount, char **arguments) return result.ended > 0 ? EXIT_HOLDERS_FOUND : EXIT_NO_HOLDERS; } - const CloseHoldersResult result = closeCockatriceHolders(holders, CLOSE_WAIT_MILLISECONDS); + // Hidden windows are asked to close here: an instance in the notification area is the one + // this update has no other way to reach, and the guard's end answer is behind that attempt. + const CloseHoldersResult result = closeCockatriceHolders(holders, CLOSE_WAIT_MILLISECONDS, true); writeLine(result.report); return result.askedToClose > 0 ? EXIT_HOLDERS_FOUND : EXIT_NO_HOLDERS; }