Skip to content

Commit

Permalink
fix(nsis): use revertible+atomic rmdir on update and add user-confirm…
Browse files Browse the repository at this point in the history
…ed retry loop (#6551)

* fix(nsis): use revertible+atomic rmdir on update and retry (w/ user confirmation)

Previously uninstaller would run `RMDir /r $INSTDIR` to remove the
currently installed version, but this operation is not atomic and if
some of the files will fail to delete it will leave them in the
directory while erasing the rest. If we are updating, however, this
leads us to a tricky situation where we cannot update these files, but
cannot also cancel the installation. Because of the erased files the app
won't be able to start if the installation won't completely at least
partially. However, the downside of that is that the app can have new
asar.unpacked files along with the old asar, executable, and bindings.

The approach of this change is to recursive use `Rename` instead of a
single `RMDir /r` in uninstaller (when `isUpdated` is true) to move the
whole app directory file by file to a temporary folder. If this
operation fails due to busy files - we use `CopyFiles` to restore all
files that we managed to move so far. Because the whole uninstallation
process becomes interrupted - the app shortcut and file associations
have to be removed only *after* the successful recursive `Rename`.

This error is caught by installer running in an update mode (see
`installUtil.nsh`) and presented to user in a dialog. If this erro
happen the installation does not proceed normally.

In addition to all of the above, this patch simplifies the last resort
measure in `extractAppPackage` which should now only run when old
uninstaller (that still uses `RMDir /r`) leaves busy files behind.

* Retry uninstall
  • Loading branch information
indutny-signal authored Jan 16, 2022
1 parent a138a86 commit 7b2a5e1
Show file tree
Hide file tree
Showing 5 changed files with 186 additions and 31 deletions.
5 changes: 5 additions & 0 deletions .changeset/chilly-trains-study.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"app-builder-lib": patch
---

fix(nsis): use revertible rmdir on update
Original file line number Diff line number Diff line change
Expand Up @@ -104,28 +104,24 @@
LoopExtract7za:
IntOp $R1 $R1 + 1

# Attempt to copy files in atomic way
CopyFiles /SILENT "$PLUGINSDIR\7z-out\*" $OUTDIR
IfErrors 0 DoneExtract7za

${if} $R1 > 1
DetailPrint `Can't modify "${PRODUCT_NAME}"'s files.`
${if} $R1 < 5
# Try copying a few times before giving up
Goto LoopExtract7za
${else}
MessageBox MB_RETRYCANCEL|MB_ICONEXCLAMATION "$(appCannotBeClosed)" /SD IDCANCEL IDRETRY RetryExtract7za
${endIf}

# CopyFiles will remove all overwritten files when it encounters an
# issue and make app non-launchable. Extract over from the archive
# ignoring the failures so at least we will partially update and the
# app would start.
Nsis7z::Extract "${FILE}"
Quit
DetailPrint `Can't modify "${PRODUCT_NAME}"'s files.`
${if} $R1 < 5
# Try copying a few times before asking for a user action.
Goto RetryExtract7za
${else}
Goto LoopExtract7za
MessageBox MB_RETRYCANCEL|MB_ICONEXCLAMATION "$(appCannotBeClosed)" /SD IDCANCEL IDRETRY RetryExtract7za
${endIf}

# As an absolutely last resort after a few automatic attempts and user
# intervention - we will just overwrite everything with `Nsis7z::Extract`
# even though it is not atomic and will ignore errors.
Nsis7z::Extract "${FILE}"
Quit

RetryExtract7za:
Sleep 1000
Goto LoopExtract7za
Expand Down
44 changes: 36 additions & 8 deletions packages/app-builder-lib/templates/nsis/include/installUtil.nsh
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,10 @@ Function handleUninstallResult
Return

${if} $R0 != 0
MessageBox MB_OK|MB_ICONEXCLAMATION "$(uninstallFailed): $R0"
DetailPrint `Uninstall was not successful. Uninstaller error code: $R0.`
SetErrorLevel 2
Quit
${endif}
FunctionEnd

Expand Down Expand Up @@ -156,7 +159,7 @@ Function uninstallOldVersion
!endif
${if} $uninstallString == ""
ClearErrors
Goto Done
Return
${endif}
${endif}

Expand All @@ -175,7 +178,7 @@ Function uninstallOldVersion
${if} $installationDir == ""
${andIf} $uninstallerFileName == ""
ClearErrors
Goto Done
Return
${endif}

${if} $installMode == "CurrentUser"
Expand Down Expand Up @@ -206,12 +209,37 @@ Function uninstallOldVersion
StrCpy $uninstallerFileNameTemp "$PLUGINSDIR\old-uninstaller.exe"
!insertmacro copyFile "$uninstallerFileName" "$uninstallerFileNameTemp"

ExecWait '"$uninstallerFileNameTemp" /S /KEEP_APP_DATA $0 _?=$installationDir' $R0
ifErrors 0 Done
# the execution failed - might have been caused by some group policy restrictions
# we try to execute the uninstaller in place
ExecWait '"$uninstallerFileName" /S /KEEP_APP_DATA $0 _?=$installationDir' $R0
Done:
# Retry counter
StrCpy $R5 0

UninstallLoop:
IntOp $R5 $R5 + 1

${if} $R5 > 5
MessageBox MB_RETRYCANCEL|MB_ICONEXCLAMATION "$(appCannotBeClosed)" /SD IDCANCEL IDRETRY OneMoreAttempt
Return
${endIf}

OneMoreAttempt:
ExecWait '"$uninstallerFileNameTemp" /S /KEEP_APP_DATA $0 _?=$installationDir' $R0
ifErrors TryInPlace CheckResult

TryInPlace:
# the execution failed - might have been caused by some group policy restrictions
# we try to execute the uninstaller in place
ExecWait '"$uninstallerFileName" /S /KEEP_APP_DATA $0 _?=$installationDir' $R0
ifErrors DoesNotExist

CheckResult:
${if} $R0 == 0
Return
${endIf}

Sleep 1000
Goto UninstallLoop

DoesNotExist:
SetErrors
FunctionEnd

!macro uninstallOldVersion ROOT_KEY
Expand Down
2 changes: 2 additions & 0 deletions packages/app-builder-lib/templates/nsis/messages.yml
Original file line number Diff line number Diff line change
Expand Up @@ -144,3 +144,5 @@ areYouSureToUninstall:
da: Er du sikker på, at du vil afinstallere ${PRODUCT_NAME}?
decompressionFailed:
en: Failed to decompress files. Please try running the installer again.
uninstallFailed:
en: Failed to uninstall old application files. Please try running the installer again.
138 changes: 131 additions & 7 deletions packages/app-builder-lib/templates/nsis/uninstaller.nsh
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,109 @@ Function un.onInit
!endif
FunctionEnd

Function un.atomicRMDir
Exch $R0
Push $R1
Push $R2
Push $R3

StrCpy $R3 "$INSTDIR$R0\*.*"
FindFirst $R1 $R2 $R3

loop:
StrCmp $R2 "" break

StrCmp $R2 "." continue
StrCmp $R2 ".." continue

IfFileExists "$INSTDIR$R0\$R2\*.*" isDir isNotDir

isDir:
CreateDirectory "$PLUGINSDIR\old-install$R0\$R2"

Push "$R0\$R2"
Call un.atomicRMDir
Pop $R3

${if} $R3 != 0
Goto done
${endIf}

Goto continue

isNotDir:
ClearErrors
Rename "$INSTDIR$R0\$R2" "$PLUGINSDIR\old-install$R0\$R2"

# Ignore errors when renaming ourselves.
StrCmp "$R0\$R2" "${UNINSTALL_FILENAME}" 0 +2
ClearErrors

IfErrors 0 +3
StrCpy $R3 "$INSTDIR$R0\$R2"
Goto done

continue:
FindNext $R1 $R2
Goto loop

break:
StrCpy $R3 0

done:
FindClose $R1

StrCpy $R0 $R3

Pop $R3
Pop $R2
Pop $R1
Exch $R0
FunctionEnd

Function un.restoreFiles
Exch $R0
Push $R1
Push $R2
Push $R3

StrCpy $R3 "$PLUGINSDIR\old-install$R0\*.*"
FindFirst $R1 $R2 $R3

loop:
StrCmp $R2 "" break

StrCmp $R2 "." continue
StrCmp $R2 ".." continue

IfFileExists "$INSTDIR$R0\$R2\*.*" isDir isNotDir

isDir:
CreateDirectory "$INSTDIR$R0\$R2"

Push "$R0\$R2"
Call un.restoreFiles
Pop $R3

Goto continue

isNotDir:
Rename $PLUGINSDIR\old-install$R0\$R2" "$INSTDIR$R0\$R2"
continue:
FindNext $R1 $R2
Goto loop
break:
StrCpy $R0 0
FindClose $R1
Pop $R3
Pop $R2
Pop $R1
Exch $R0
FunctionEnd
Section "un.install"
# for assisted installer we check it here to show progress
!ifndef ONE_CLICK
Expand All @@ -38,6 +141,34 @@ Section "un.install"
!insertmacro setLinkVars
# delete the installed files
!ifmacrodef customRemoveFiles
!insertmacro customRemoveFiles
!else
${if} ${isUpdated}
CreateDirectory "$PLUGINSDIR\old-install"
Push ""
Call un.atomicRMDir
Pop $R0
${if} $R0 != 0
DetailPrint "File is busy, aborting: $R0"
# Attempt to restore previous directory
Push ""
Call un.restoreFiles
Pop $R0
Abort `Can't rename "$INSTDIR" to "$PLUGINSDIR\old-install".`
${endif}
${endif}
# Remove all files (or remaining shallow directories from the block above)
RMDir /r $INSTDIR
!endif
${ifNot} ${isKeepShortcuts}
WinShell::UninstAppUserModelId "${APP_ID}"
Expand All @@ -64,13 +195,6 @@ Section "un.install"
!insertmacro unregisterFileAssociations
!endif
# delete the installed files
!ifmacrodef customRemoveFiles
!insertmacro customRemoveFiles
!else
RMDir /r $INSTDIR
!endif

Var /GLOBAL isDeleteAppData
StrCpy $isDeleteAppData "0"
Expand Down

0 comments on commit 7b2a5e1

Please sign in to comment.