search

microsoft / PowerToys

Windows system utilities to maximize productivity
MIT License
109.71k stars 6.46k forks source link

Installer crashes when file is named "PowerToys.exe" #27725

Open ByteSizedMarius opened 1 year ago

ByteSizedMarius commented 1 year ago

Microsoft PowerToys version

0.72.0

Installation method

GitHub

Running as admin

No

Area(s) with issue?

Installer

Steps to reproduce

✔️ Expected Behavior

Installer to run

❌ Actual Behavior

Installer opens hundreds of "Are you sure you want to cancel"-popups, temporarily rendering the machine unusable. Installer needs to be killed via taskmgr.

Other Software

No response

yuyoyuppe commented 1 year ago

Thanks for the report. I've reproduced the bug, it reminded me of the old Solitaire victory animation 🙂

The reason it happens is that we try to terminate PowerToys.exe before beginning the installation, so the fix would be to check against trying to terminate our own process.

yuyoyuppe commented 1 year ago

Actually, we have multiple places which try to kill the process:

  1. <util:CloseApplication in PowerToys.wxs
  2. terminate_powertoys.cmd that's also called from PowerToys.wxs. It's the one responsible for the message boxes. We have it because of #20430.
  3. TerminateProcessesCA which is performed before installation.

Let's see what we can do about them:

  1. is there for legacy reasons and could be removed.
  2. exists because before https://github.com/microsoft/PowerToys/pull/22217, we needed to kill PowerToys.exe process to be able to install dotnet. It was implemented as a batch script because WiX bootstrapper doesn't support C++ custom actions. Now that https://github.com/microsoft/PowerToys/pull/22217 is merged, I believe 2. could be removed as well.
  3. is a CA, and is hosted by a msiexec.exe process that's not a child process of our installer process. That means there's no easy way to determine the original installer process PID.

So even if we get rid of 1. & 2., it'd be a bit untrivial to fix 3. for this particular use-case.

I think we'll mark it as a known issue for now unless more people need to rename the installer to PowerToys.exe for some reason.