Upgrade flags --all and --unninstall-previous naively includes own running shell

[marcosdly]

Brief description of your issue

Running winget upgrade with flags --all and --uninstall-previous will include running shell's package, leaving it's own and any other instances to run out of RAM or crash. If shell can run out of RAM, programs like neovim which rely heavily on i/o feedback become unpredictable and unusable. winget upgrade is interrupted without notice.

Steps to reproduce

Using pwsh.exe.

Shell session 1

pwsh.exe  # shell that can run from RAM
nvim      # any i/o heavy program

Shell session 2

pwsh.exe  # open installed, out-of-date shell
winget upgrade  # assert at least 2 packages and Microsoft.Powershell is not last
winget upgrade --all --uninstall-previous # will remove own shell amid runtime

Then

• Try interacting with shell session 1.
• Try opening another pwsh.exe instance.
• (lastly) Try interacting with shell session 2.

Expected behavior

Error, unskipable warning prompt, or unsafe flag to prevent or force explicit action.

Actual behavior

• Uninstalls shell it is running in without question.
• Process terminates/halts without notice.
• Shell may crash or become unusable.
• Scrambled/bad data at other sessions of same uninstalled shell.
• No warnings are issued.
• Package's executable is not compared with own shell's executable.

Environment

Windows Package Manager v1.10.340
Copyright (c) Microsoft Corporation. All rights reserved.

Windows: Windows.Desktop v10.0.19045.5608
System Architecture: X64
Package: Microsoft.DesktopAppInstaller v1.25.340.0

Winget Directories
-------------------------------------------------------------------------------------------------------------------------------
Logs                               %LOCALAPPDATA%\Packages\Microsoft.DesktopAppInstaller_8wekyb3d8bbwe\LocalState\DiagOutputDir
User Settings                      %LOCALAPPDATA%\Packages\Microsoft.DesktopAppInstaller_8wekyb3d8bbwe\LocalState\settings.json
Portable Links Directory (User)    %LOCALAPPDATA%\Microsoft\WinGet\Links
Portable Links Directory (Machine) C:\Program Files\WinGet\Links
Portable Package Root (User)       %LOCALAPPDATA%\Microsoft\WinGet\Packages
Portable Package Root              C:\Program Files\WinGet\Packages
Portable Package Root (x86)        C:\Program Files (x86)\WinGet\Packages
Installer Downloads                %USERPROFILE%\Downloads
Configuration Modules              %LOCALAPPDATA%\Microsoft\WinGet\Configuration\Modules

Links
---------------------------------------------------------------------------
Privacy Statement   https://aka.ms/winget-privacy
License Agreement   https://aka.ms/winget-license
Third Party Notices https://aka.ms/winget-3rdPartyNotice
Homepage            https://aka.ms/winget
Windows Store Terms https://www.microsoft.com/en-us/storedocs/terms-of-sale

Admin Setting                             State
--------------------------------------------------
LocalManifestFiles                        Disabled
BypassCertificatePinningForMicrosoftStore Disabled
InstallerHashOverride                     Disabled
LocalArchiveMalwareScanOverride           Disabled
ProxyCommandLineOptions                   Disabled
DefaultProxy                              Disabled

[denelon]

There isn't a reliable intrinsic method for WinGet to know what shell it's being executed inside of. Several packages are set to "deferred registration" by default, but in some cases it's not possible. Passing arguments to WinGet like "--uninstall-previous" are an explicit action telling WinGet what to do as opposed to the "default" behavior.

I'm not exactly sure what we could or should do in a case like the one mentioned above, but I'm happy to leave the issue open if others have interesting ideas that might provide a better experience.

[marcosdly]

@denelon Maybe walk up the parent processes WinGet is running under, then assert the root directory for the package being uninstalled/overwitten is not relative to the path of any executable that is a parent of WinGet. In pseudo code, it would be like:

parent_executables: list[Path] = get_parent_exe_recursive(start_at=self, stop_at='explorer.exe')

function removing_will_kill_self(package_dir: Path) -> bool:
  for parent_exe in parent_executables do
    if parent_exe.starts_with(package_dir) then
      return True
  return False

Moreover, Process Explorer and third-party software like resmon can grep process info without asking for admin permission. Process Explorer can even display them as a tree better than Task Manager.

Process Explorer is very fast at updating process info. Perhaps it could even be feasible to fetch/update the process tree lookup at each such check. I imagine this could only be a problem in extremely low I/O throughput environments, like some (very) bad C:\ disk drives I dared to run Windows 10 in, or overwhelmed systems.

[Trenly]

One thing that makes this more difficult is that WinGet doesn't know what path(s) an installer will touch during the installation. Effectively, WinGet downloads the installer and executes it with whatever command line parameters are needed for the installation, or, in the case of portable packages or MSIX packages, uses Shell APIs to install it. For any other given executable or MSI package, WinGet doesn't actually control the installation process - that is entirely contained within the installer that is downloaded, just as if you were to download it from the internet yourself.

For your use case, take neovim for example; you should be able to see similar behavior if you download the MSI file for a new version of neovim, launch your existing version of neovim, and then use msiexec /i <path-to-downloaded-file> /passive /qb /norestart

[marcosdly]

@Trenly Can installers be dry ran or gracefully cancelled/halted?

If so, one could:

1. Store thread ids;
2. Use filesystem watch or change journal to mutex write operations under the child thread;
3. Lock;
4. Check files owned/modified by thread (before performing action, somehow);
5. Compare against parent processes path (as cited before);
6. Perform action:

• Gracefully cancel (unsafe); or
• Halt and prompt user for killing all instances (explicit); or
• Watch until zero instances of the package are running (safe, requires explicit warning/info user notification).

7. Unlock; Repeat.

Watching filesystem changes

Specifically under a process/thread

SHChangeNotifyRegister is very precise at that, if the filesystem supports notifications. Though it takes a HWND argument, which I assume works only with GUI windows?

Volume-wide

Change Journals are admittedly expensive:

The brute force method of checking for changes in directories or files is to scan the entire volume.

However, this is often not an acceptable approach because of the decrease in system performance it would cause.

Scope-wide (sub-tree of filesystem path)

ReadDirectoryChangesW and FindFirstChangeNotificationA would also not be acceptable because even if they were useful, the broad scope of changes would require checks to be done volume-wise unless a way of filtering possibly relevant subtrees of those volumes can be guessed.

Application events

Logs, hijacked output streams, etc?

Basically, standard output actions performed by standard installers, like MSI. The logging of which files are being altered in the installer's GUI seems to be consistent as a string format.

[marcosdly]

CreateEventA documentation states:

A child process created by the CreateProcess function can inherit a handle to an event object if the lpEventAttributes parameter of CreateEvent enabled inheritance.

A process can specify the event-object handle in a call to the DuplicateHandle function to create a duplicate handle that can be used by another process.

Just laying down the options to the best of my knowledge. I'm no win32 developer. Waiting for judgement.

[whindsaks]

SHChangeNotifyRegister is very precise at that, if the filesystem supports notifications. Though it takes a HWND argument, which I assume works only with GUI windows?

A console application can create a hidden window. However, this shell notification function is not reliable when there are many changes, it just gives up and tells you a generic "something in this directory changed".

[marcosdly]

@whindsaks If the installer spawns a window, it may be good enough. A directory filter attached to the installer's process is a performance improvement and a reliable guess.

WinGet wouldn't check files anyway. If something changes in the installation directory of a running process, a package manager would assume that's dangerous. Plus, we have a lookup-list of relevant directories.

EDIT: An installer is a child process of WinGet.

WinGet downloads the installer and executes it with whatever command line parameters are needed for the installation, or, in the case of portable packages or MSIX packages, uses Shell APIs to install it.

[marcosdly]

A filter could be: if running exe is in PATH, use the deepest/highest relative directory.

Let exe be C:\a\b\c\d\e.exe:

$env:Path += C:\a      # highest
$env:Path += C:\a\b\c  # deepest

[AltitudeApps]

This seems like a pretty big ask, but I agree with the sentiment that this would be useful to have.

This same sort of file system activity monitoring would also be nice to be able to provide a report on exactly what happened to my file system, and where it happened, as a result of an installer process being invoked.

Subscribing to this thread so I'll be informed of any developments, as this functionality would be useful in the scenario I just described.