Uninstalling leaves the rootAppDirectory behind if there is an in-use shell extension in it.

[theofanis]

Obviously, since if the extension is loaded with explorer.exe before uninstalling, then it's in use so Squirrel fails to delete it, thus leaving the app install directory behind with the extension dll in it.

Of course, this happens even when unregistering the dll on uninstall, because of the above reason.

Refreshing the explorer does not work, as expected. Now you can tell me that I could kill and restart explorer.exe, but that would close any windows the user may have opened before. Imaging killing explorer.exe when a file move is ongoing.

Any ideas how this could be handled better?

[anaisbetts]

It should get cleaned up on the next reboot, does it not?

[theofanis]

How did I forget about that?

I'm between dev/testing sessions and completely forgot about it. Sorry for the false alarm!

Edit: It appears that I have deleted the folder on my own last night, excuse the confusion please. See comments below.

[theofanis]

I'm reopening this.

Just tested again and on another PC and the root dir with Update.exe didn't get deleted upon rebooting. Btw the same happens with the new Slack for windows beta app.

I'm uninstalling through the Programs and Features in Control Panel.

[theofanis]

After going through squirrel's code once more, I see MoveFileEx with MOVEFILE_DELAY_UNTIL_REBOOT flag is used to do this.

AFAIK, MoveFileEx, which is used for this in Utility class, needs elevated privileges to write to HKLM. Check documentation and especially remarks here:

If the dwFlags parameter specifies MOVEFILE_DELAY_UNTIL_REBOOT, MoveFileEx fails if it cannot access the registry. The function stores the locations of the files to be renamed at restart in the following registry value:
HKEY_LOCAL_MACHINE\SYSTEM\CurrentControlSet\Control\Session Manager\PendingFileRenameOperations

I don't see this key in my registry.

Since my app, and slack for that matter, don't need elevated privileges, and when uninstalling I don't see the UAC dialog, MoveFileEx obviously fails.

I see that Utility.DeleteDirectoryAtNextReboot does things in a correct order, it marks files in top level and in subdirectories for removal and then marks subdirectories and top directory for removal. I also see the comment that says this will blow up if the user is non-admin.

Since, Squirrel advertises that it doesn't prompt any UAC dialogs, why is that there, since it obviously won't work for any non-admin user.

Btw, interesting article about MoveFileEx usage with an interesting comment:

Rename the DLL file first (coolcorelibrary.old), and then mark it for deletion. You can rename a DLL while loaded into a process, and applications will continue using the original DLL until it is unloaded, while any new applications will use the new one.

Bottom line: Should there be a way to run Update.exe as administrator? And I don't mean manually from the command line, since runas can do that, or an elevated command prompt could overcome this. But to run Update.exe as administrator by clicking Uninstall in Programs and Features, which checks the UninstallString in HKCU\Software\Microsoft\Windows\CurrentVersion\Uninstall\AppName key to locate the uninstaller.

[theofanis]

OK adding one more undesired effect complementary to the above.

Since my in-use dll remains in the app folder, if I choose to reinstall the app, the installation fails, since it tries to delete the "dead" install folder and it cannot do that, since the dll remains in use.

I guess renaming the app root directory (append '.old'?) when it can't deleted could overcome this. Of course the rename should take place inside Utility.DeleteDirectoryAtNextReboot, exactly before trying to safely remove its files and subdirectories.

[willdean]

I think renaming is a good idea, but it may be better to do something a little more than just appending 'old', because that only kicks the problem one stage down the road, in that you've now got one extra install before a reboot, but no more after that. Appending something like a GUID is a quick and ugly way to generate a directory which is likely to stay uniquely named, but obviously there are lots of other approaches. (Please though, not the unicode letter thing which the WithTempDirectory code uses..!)

[theofanis]

You're right, appending '.old' could cause problems if the uninstall/reinstall cycle happens more than once before rebooting, so GUID sounds just fine.

Having an optional bool rename parameter for Utility.DeleteDirectoryWithFallbackToNextReboot could do the job, since you probably don't want to rename subdirectories as well, just the top root app directory. Let it default to true, as to not affect the two points its being called (aside from its recursion) and pass false to the recursive call.

[willdean]

I'm with your earlier comments that the whole MOVEFILE_DELAY_UNTIL_REBOOT strategy seems a source of confusion in a project which is intended to run without elevation. (See for example last comment on #221)

It might be better to replace the whole 'until reboot' strategy with a systematic renaming approach, which just moved old directories into easily identified 'zombie' names. There are then lots of opportunities to clean these up at some future point - e.g. during the next update, or next time update.exe is used as the launcher from the app, whatever.

If we really wanted to have them gone as soon as possible, then we could have a cleanup invocation of update.exe put into the HKCU runonce key, so that they disappeared on the next boot once the same user had logged back in.

[theofanis]

I'm also thinking that this strategy shouldn't exist inside Update.exe. Maybe in another utility which could be added in RunOnce. After all, that's why it exists, to help finish install/uninstall operations.

Update.exe could leave behind a file containing each path to be removed. The utility can delete (or try for that matter) everything found in there and self-delete with a batch file maybe.

The batch file approach is what I currently do when uninstalling my squirrel aware app in order to leave nothing behind, but the need for the shell extension arose and here we are. :)

[anaisbetts]

If the dwFlags parameter specifies MOVEFILE_DELAY_UNTIL_REBOOT, MoveFileEx fails if it cannot access the registry. The function stores the locations of the files to be renamed at restart in the following registry value:

:-/ Of course it does, Thanks Windows

[willdean]

Raymond Chen has probably long ago written some condescending blog post explaining why this is so...