From a83d82b5e82c5801fc5b9b5e7bd33d191409a836 Mon Sep 17 00:00:00 2001 From: "sentry[bot]" <39604003+sentry[bot]@users.noreply.github.com> Date: Wed, 28 Oct 2026 09:08:54 +0100 Subject: [PATCH] [Installer] Recover leftover payload files when previous uninstaller leaves them behind (#7676) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * [Installer] Recover leftover payload files when previous uninstaller leaves them behind * [Installer] Fix leftover recovery on the 64-bit auto-uninstall path - Record the 64-bit uninstaller exit code after ExecWait returns. It was captured before the call (always empty), so WaitForOldPayloadToDisappear always aborted before RemoveOldPayloadLeftovers could run. - Restore the pre-uninstall user-data backup before aborting out of the wait, including the guard's cancel and probe-failure prompts, so the only copy cannot be stranded in %TEMP% for the next run to wipe. - Report the leftover removal as done only when every payload executable is gone and RetryDllPurge reports no stale DLLs, instead of checking cockatrice.exe alone. - Return directly after a successful recovery instead of reusing the poll counter, so the log stops claiming the directory "was already clear" and the normal settle delay is kept. * [Installer] Delete the old uninstaller only after recovery succeeds RemoveOldPayloadLeftovers cleared the attribute and deleted $INSTDIR\uninstall.exe on every attempt, before the attempt was known to work. On failure the installer aborts telling the user to run it again, but UninstallString then points at a missing uninstaller: ExecWait leaves the exit code empty, WaitForOldPayloadToDisappear takes the "cancelled or failed" branch, and RemoveOldPayloadLeftovers is gated on exit code 0, so no later run can ever get past it - a half-removed old install the previous revision never produced. Delete it in rop_done instead, after $OldPayloadRemoved is set. WriteUninstaller recreates it on success anyway, and nothing in the success verdict depends on it being gone. --------- Co-authored-by: sentry[bot] <39604003+sentry[bot]@users.noreply.github.com> Co-authored-by: Lukas Brübach --- cmake/NSIS.template.in | 158 +++++++++++++++++++++++++++++++++++++++-- 1 file changed, 154 insertions(+), 4 deletions(-) diff --git a/cmake/NSIS.template.in b/cmake/NSIS.template.in index 6b861a370..cda370670 100644 --- a/cmake/NSIS.template.in +++ b/cmake/NSIS.template.in @@ -15,6 +15,17 @@ Var ReinstallMode Var GuardDir Var OldUninstallString Var OldInstallDir +; What ExecWait reported for the previous version's uninstaller (empty when it could not be +; started at all). Read by WaitForOldPayloadToDisappear: an uninstaller that ran in place +; (_?=) and failed or was cancelled will not remove anything more, however long we wait. +Var OldUninstallExit +; Set to 1 by RemoveOldPayloadLeftovers only when every leftover it is responsible for is +; gone; WaitForOldPayloadToDisappear aborts while it is still 0. +Var OldPayloadRemoved +; Scratch state of AppendUninstallerLog. +Var UninstLogHandle +Var UninstLogLine +Var UninstLogChar Var DataBackupDir Var DataBackupDone Var DataBackupForDir @@ -198,6 +209,7 @@ StrCpy $LooseDeckNoticeShown "" ; No old install location resolved yet; Backup/Restore fall back to $INSTDIR ; when this is empty (the /R path where $INSTDIR already is the old dir). StrCpy $OldInstallDir "" +StrCpy $OldUninstallExit "" ; No purge has run and nothing has been measured yet. The section sets both ; after a purge, or measures the directory itself when neither purge applies. StrCpy $StaleRuntimeAfterPurge 0 @@ -404,7 +416,9 @@ StrCpy $OldInstallDir $INSTDIR StrCmp $R0 "" done32 DetailPrint "Removing previous version (32-bit)..." Call BackupUserDataBeforeUninstall +StrCpy $R5 "" ExecWait '$R0 _?=$OldInstallDir' $R5 +StrCpy $OldUninstallExit $R5 !insertmacro CockLog "uninstall (32-bit): exit=$R5" Call WaitForOldPayloadToDisappear Call RestoreUserDataAfterUninstall @@ -419,9 +433,11 @@ ${If} ${RunningX64} StrCpy $OldInstallDir $INSTDIR !insertmacro CockLog "uninstall (64-bit): '$R0 _?=$OldInstallDir'" StrCmp $R0 "" done64 + StrCpy $R5 "" DetailPrint "Removing previous version (64-bit)..." Call BackupUserDataBeforeUninstall ExecWait '$R0 _?=$OldInstallDir' $R5 + StrCpy $OldUninstallExit $R5 !insertmacro CockLog "uninstall (64-bit): exit=$R5" Call WaitForOldPayloadToDisappear Call RestoreUserDataAfterUninstall @@ -439,6 +455,12 @@ FunctionEnd ; ExecWait returns at once while the copy keeps deleting - and it finishes after this ; window has closed, removing the version that was just installed. Waiting for ; cockatrice.exe to disappear covers both cases, and stops with a message instead of +; +; When the payload is still there after the wait, $OldUninstallExit decides: a non-zero +; code means the uninstaller was cancelled or failed, and the install aborts saying so. +; A zero code means it finished but skipped files someone was holding (NSIS Delete does +; that silently), so RemoveOldPayloadLeftovers frees and removes them, and the install +; only aborts - naming the holders - when even that fails. ; installing over a directory that is still being emptied. Function WaitForOldPayloadToDisappear ${If} $OldInstallDir == "" @@ -449,10 +471,37 @@ old_uninstall_wait: IfFileExists "$OldInstallDir\cockatrice.exe" 0 old_uninstall_gone IntOp $R2 $R2 + 1 ${If} $R2 > 60 - !insertmacro CockLog "old uninstall: $OldInstallDir still holds cockatrice.exe after 30 s" - StrCpy $AbortReason \ - "The previous version in $OldInstallDir is still there after its uninstaller ran.$\r$\n$\r$\nIf that uninstaller was cancelled or reported an error, close anything still using those files and run this installer again." - Call AbortInstall + !insertmacro CockLog "old uninstall: $OldInstallDir still holds cockatrice.exe after 30 s (uninstaller exit=$OldUninstallExit)" + ; The uninstaller's own account of what it skipped or why it stopped lives in its + ; separate log; copy it here so the report sent from AbortInstall carries it. + Call AppendUninstallerLog + ${If} $OldUninstallExit != "0" + ; Cancelled, failed, or could not be started at all (empty code): the old version + ; was not meant to be removed, so nothing is deleted on its behalf. + StrCpy $AbortReason \ + "The previous version in $OldInstallDir is still there because its uninstaller was cancelled or failed (exit code '$OldUninstallExit').$\r$\n$\r$\nClose anything still using those files and run this installer again." + ; The backup was taken before the uninstaller ran; an abort must not leave the + ; only copy of the user data in %TEMP%, where the next run clears it. + Call RestoreUserDataAfterUninstall + Call AbortInstall + ${EndIf} + ; The uninstaller finished but skipped files - a running instance or a scanner holding + ; them, which NSIS Delete passes over silently. Free them and remove what is left. + Call RemoveOldPayloadLeftovers + ; The removal reports success only when every payload executable is gone and the + ; DLL/Plugins purge came back clean; cockatrice.exe alone would hide a locked + ; oracle.exe or a stale DLL the new payload is about to be written over. + ${If} $OldPayloadRemoved != "1" + StrCpy $AbortReason \ + "The previous version in $OldInstallDir could not be fully removed after its uninstaller ran.$\r$\n$\r$\nStill in use by: $HolderOutput$\r$\n$\r$\nClose anything still using those files and run this installer again." + Call RestoreUserDataAfterUninstall + Call AbortInstall + ${EndIf} + ; The payload is gone and ExecWait already waited for the uninstaller; keep the + ; short settle the other paths use, then hand back to the caller's restore. + Sleep 1000 + !insertmacro CockLog "old uninstall: continuing after removing the leftovers the uninstaller skipped" + Return ${EndIf} Sleep 500 Goto old_uninstall_wait @@ -467,6 +516,97 @@ ${If} $R2 > 0 ${Else} !insertmacro CockLog "old uninstall: $OldInstallDir was already clear when ExecWait returned" ${EndIf} +Return +FunctionEnd + +; Called when the old uninstaller exited successfully but left its payload behind. +; Frees the files the same way the guard does (close, then offer to end), deletes what +; the uninstaller skipped, and on failure records which files survived and who holds +; them in $HolderOutput, for the abort message. Sets $OldPayloadRemoved to 1 only when +; every payload executable is gone and the DLL/Plugins purge is clean. +Function RemoveOldPayloadLeftovers + !insertmacro CockLog "old uninstall: removing leftovers of $OldInstallDir" + StrCpy $OldPayloadRemoved 0 + StrCpy $HolderOutput "nothing could be named" + StrCpy $GuardDir $OldInstallDir + Call GuardAppsNotRunning + ; The purge helpers work on $INSTDIR; point them at the old directory for the + ; duration (on /R the two are the same anyway). + Push $INSTDIR + StrCpy $INSTDIR $OldInstallDir + StrCpy $R2 0 + rop_loop: + SetFileAttributes "$INSTDIR\cockatrice.exe" NORMAL + SetFileAttributes "$INSTDIR\oracle.exe" NORMAL + SetFileAttributes "$INSTDIR\servatrice.exe" NORMAL + Delete "$INSTDIR\cockatrice.exe" + Delete "$INSTDIR\oracle.exe" + Delete "$INSTDIR\servatrice.exe" + Delete "$INSTDIR\qt.conf" + Call RetryDllPurge + ; $R0 is RetryDllPurge's verdict (1 = a DLL or the Plugins directory survived). + ; Every payload executable has to be gone as well: a locked oracle.exe or + ; servatrice.exe is just as fatal to the install that follows as cockatrice.exe. + IfFileExists "$INSTDIR\cockatrice.exe" rop_retry + IfFileExists "$INSTDIR\oracle.exe" rop_retry + IfFileExists "$INSTDIR\servatrice.exe" rop_retry + ${If} $R0 == 0 + Goto rop_done + ${EndIf} + rop_retry: + IntOp $R2 $R2 + 1 + ${If} $R2 < 3 + !insertmacro CockLog "old uninstall: leftovers of $OldInstallDir survived removal attempt $R2, retrying in 2 s" + Sleep 2000 + Goto rop_loop + ${EndIf} + !insertmacro CockLog "old uninstall: leftovers of $OldInstallDir still present after 3 removal attempts" + !insertmacro CockLogListing "old-leftovers" "*.*" + ; Per-file Windows codes for the DLLs plus the holder probe's line ($HolderOutput). + Call LogStaleRuntimeDiagnostics + Pop $INSTDIR + Return + rop_done: + StrCpy $OldPayloadRemoved 1 + ; Only now that the recovery has succeeded: deleting the old uninstaller on a + ; failed attempt would leave a directory whose UninstallString points at nothing, + ; so no later run could remove it. WriteUninstaller recreates it on success anyway. + SetFileAttributes "$INSTDIR\uninstall.exe" NORMAL + Delete "$INSTDIR\uninstall.exe" + !insertmacro CockLog "old uninstall: leftovers of $OldInstallDir removed" + !insertmacro CockLogListing "old-leftovers-after-removal" "*.*" + Pop $INSTDIR +FunctionEnd + +; Copies the old uninstaller's log ($TEMP\Cockatrice-uninstaller.log) into this +; installer's log, so the report AbortInstall sends shows what the uninstaller did. +; Uninstallers older than this log simply have no file; that is logged too. +Function AppendUninstallerLog + ClearErrors + FileOpen $UninstLogHandle "$TEMP\Cockatrice-uninstaller.log" r + ${If} ${Errors} + ${OrIf} $UninstLogHandle == "" + !insertmacro CockLog "uninstaller log: not found at $TEMP\Cockatrice-uninstaller.log" + Return + ${EndIf} + !insertmacro CockLog "uninstaller log begin ----------------------------------------" + aul_loop: + ClearErrors + FileRead $UninstLogHandle $UninstLogLine + IfErrors aul_done + ; Strip the line break FileRead keeps, so CockLog does not double it. + aul_trim: + StrCpy $R9 $UninstLogLine 1 -1 + ${If} $R9 == "$\n" + ${OrIf} $R9 == "$\r" + StrCpy $UninstLogLine $UninstLogLine -1 + Goto aul_trim + ${EndIf} + !insertmacro CockLog "uninstaller: $UninstLogLine" + Goto aul_loop + aul_done: + FileClose $UninstLogHandle + !insertmacro CockLog "uninstaller log end ------------------------------------------" FunctionEnd ; BackupUserDataBeforeUninstall / RestoreUserDataAfterUninstall @@ -1000,6 +1140,9 @@ Function GuardAppsNotRunning !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." + ; RemoveOldPayloadLeftovers runs with the pre-uninstall backup already taken; + ; restoring here keeps this abort from stranding the only copy in %TEMP%. + Call RestoreUserDataAfterUninstall Call AbortInstall ; Only reached through Yes: AbortInstall does not return. guard_end: @@ -1031,6 +1174,9 @@ Function GuardAppsNotRunning guard_unknown_abort: StrCpy $AbortReason \ "The installation was cancelled because it could not check whether Cockatrice, Oracle or Servatrice is still running.$\r$\n$\r$\nThe check reported: $GuardOutput$\r$\n$\r$\nPlease close them and run the installer again." + ; Same reason as the cancel path above: this abort can be reached from the + ; leftover-removal guard, after the backup was taken. + Call RestoreUserDataAfterUninstall Call AbortInstall FunctionEnd @@ -1818,7 +1964,9 @@ ${If} $PortableMode = 0 !insertmacro CockLog "componentsPagePre: old 32-bit uninstall '$OldUninstallString _?=$OldInstallDir' (oldDir=$OldInstallDir InstDir=$INSTDIR)" ClearErrors Call BackupUserDataBeforeUninstall + StrCpy $R5 "" ExecWait "$OldUninstallString _?=$OldInstallDir" $R5 + StrCpy $OldUninstallExit $R5 !insertmacro CockLog "componentsPagePre: old 32-bit uninstall exit=$R5" Call WaitForOldPayloadToDisappear Call RestoreUserDataAfterUninstall @@ -1854,7 +2002,9 @@ ${If} $PortableMode = 0 !insertmacro CockLog "componentsPagePre: old 64-bit uninstall '$OldUninstallString _?=$OldInstallDir' (oldDir=$OldInstallDir InstDir=$INSTDIR)" ClearErrors Call BackupUserDataBeforeUninstall + StrCpy $R5 "" ExecWait "$OldUninstallString _?=$OldInstallDir" $R5 + StrCpy $OldUninstallExit $R5 !insertmacro CockLog "componentsPagePre: old 64-bit uninstall exit=$R5" Call WaitForOldPayloadToDisappear Call RestoreUserDataAfterUninstall