Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean out the SquirrelTemp folder #1195

Closed
Mottie opened this issue Nov 3, 2017 · 23 comments
Closed

Clean out the SquirrelTemp folder #1195

Mottie opened this issue Nov 3, 2017 · 23 comments

Comments

@Mottie
Copy link

Mottie commented Nov 3, 2017

I was analyzing my SSD today and found that my c:\Users\{user}\AppData\Local\SquirrelTemp folder had over 2 GB of "temp" files. I deleted them all, darn the repercussions, because I needed the room. Is there something in place to clean up this folder? I don't want to remember to check on it every month or so and have to clean it out.

@anaisbetts
Copy link
Contributor

This folder cleans itself up, but if you had a lot of failed installations / upgrades that might not happen. There are no repercussions to deleting this folder, sorry about the disk space

@hl2guide
Copy link

hl2guide commented Apr 8, 2021

This continues to be an issue for me. I'm using GitHub Desktop (uses Squirrel) and I get a ridiculous number of "temp" folders with over 6GB of files.

SquirrelTemp folder
Size: 6.22 GB (6,683,602,805 bytes)
Contains: 155,151 Files, 5,203 Folders

Please improve the cleanup and the update attempt logic.

image

@sergiou87
Copy link

Out of curiosity @hl2guide, do you know if all those folders belong specifically to GitHub Desktop updates and not to any other apps?

@Bluespide
Copy link

Yes, same for me. Every folder looks like this:
image

@2play
Copy link

2play commented Oct 6, 2021

Same here 1.2GB
image

@Whathecode
Copy link

Whathecode commented Oct 8, 2021

13 GB for me. So much for "temp".

image

Essentially all of these seem due to GitHub Desktop. I filed a bug report to them as well. (Still an awesome Git client regardless.)

I guess what I'm looking at here is an historical overview of Github Desktop application size over the course of the past 4-5 years. 😂

@hydraxic
Copy link

Same for me, my laptop has very little storage to begin with and I had over 177k files from that folder.

@Sappurit
Copy link

Sappurit commented Aug 10, 2022

2.5 GB since Mar 2022
snap6615

@anaisbetts
Copy link
Contributor

Once again, this is due to files being locked - given that this only happens to GitHub Desktop I would guess that they need to change something

@hl2guide
Copy link

@sergiou87 yes, they are all GitHub Desktop temps.

@sergiou87
Copy link

@anaisbetts thanks for the hint! I've tried to reproduce the issue by locking different files, and in all cases the two temp folders are deleted. If the locked file is the "execution stub", I see a line in the logs for it:

LogHost.Default.WarnException("Can't write execution stub, probably in use", e);

In general, looking at the code (and unless I'm missing something), seems like both folders should be deleted after a failed installation in any case, since the two temp folders created are in a using statement:

using (Utility.WithTempDirectory(out baseTempPath, null))
using (Utility.WithTempDirectory(out tempPath, null)) {

However, I was able to repro the problem with this other hint from one of our users: if I close GitHub Desktop while Squirrel.Windows is either preparing or installing the release, those temp folders are left there forever. I'm not an expert in C#, but I guess using statements don't dispose the resource if the app is closed. I even got a corrupted installation at some point because I closed the app while the Squirrel was copying the files to the destination folder 😢

Can you think of a proper way of fixing this from Squirrel.Windows? Otherwise, from GitHub Desktop I can only think of preventing the user from closing the app while an update is being installed 🤔

@caesay
Copy link

caesay commented Aug 16, 2022

My fork of Squirrel has solved this particular issue in two ways:

  • Squirrel temporary files are extracted to %temp% instead of %localappdata%. This means if there are any files left behind, it will be cleaned up automatically by windows (if this is configured via windows settings) or will be cleaned up when running any disk space cleaning utility.
  • Squirrel files are also cleaned up by Squirrel itself, on subsequent runs. When squirrel is installing or updating any app, if it finds any old squirrel temp directories they are eradicated.

Of course, in most cases, neither of the above are necessary. I have taken great care to eagerly clean up temporary files as soon as possible, but it feels good to know that if something does go drastically wrong, those files will still eventually get cleaned up.

@anaisbetts
Copy link
Contributor

anaisbetts commented Aug 16, 2022

Squirrel temporary files are extracted to %temp% instead of %localappdata%. This means if there are any files left behind, it will be cleaned up automatically by windows (if this is configured via windows settings) or will be cleaned up when running any disk space cleaning utility.

Yikes, have you tried this in production? Temp is often an Extremely Cursed directory (missing, pointing to something insane, broken permissions, the env var being pointed to something like "@#$I@#J", the list goes on). Also, from an antivirus perspective, you are extracting PE files to Temp then attempting to move them into place, which is nearly indistinguishable from malware

@caesay
Copy link

caesay commented Aug 16, 2022

Interesting point of view, and worth contemplating. Of course I'm not just using the environment variable, but it's being used in production by myself and a few very large open source projects, so probably hundreds of thousands of installs if not more all over the world and I've not heard anything about this being an issue. Occasionally get reports of the installer failing in weird ways because it ran out of disk space (so we now check this before extracting anything), and we had a few unicode problems in other parts of the world that are now resolved.

Even if the Squirrel temp files were still in app data, having Squirrel clean them up / reuse folder names at next install/update also solves this problem.

@endolith
Copy link

1 GB of "temp" files for me, GitHub Desktop was installed

@sergiou87
Copy link

Hopefully, desktop/desktop#15416 will help with this. We think this happens because GitHub Desktop is closed while it's updating the app, and the temp installation files are left there forever 😕

@anaisbetts
Copy link
Contributor

anaisbetts commented Oct 24, 2022

Actually to be honest @sergiou87, I would instead guess that you're trying to call quitAndInstallUpdate when there's an update available and the app is going to quit anyways. However, one of the reasons that an app is quitting is that the machine is shutting down, which would cause exactly This to happen.

Instead, it would be better if you ran the update in the background, when the app is idle (Electron's autoUpdater is heavily based on Squirrel.Mac which unfortunately can't do this, but you could invoke update.exe directly).

@sergiou87
Copy link

I would instead guess that you're trying to call quitAndInstallUpdate

But if I do that, it will reopen the app, no? 🤔 In that scenario, the user is waiting for the app to install the update and close itself, and IIRC when the app is opened again, the new version will be installed, right?

However, one of the reasons that an app is quitting is that the machine is shutting down, which would cause exactly This to happen

Interesting, thanks! I'll look into this, because I didn't consider that scenario 😨

However, running the update in the background wouldn't help in this situation, right? It'd be exactly the same problem, unless I'm missing something 🤔 At least, the problem I've seen is when the app is closed after the update has been downloaded and before the update-downloaded is emitted, because in the meantime it's actually installing the update and even replacing some files like the GitHubDesktop.exe.

The app already checks for updates every 4h, and immediately downloads them if available (to then wait for users to quit / restart the app). Does that make sense?

@anaisbetts
Copy link
Contributor

However, running the update in the background wouldn't help in this situation, right?

It would, because if you're idling you're vaguely sure that the machine isn't shutting down, whereas if you're doing it on app close, that is actually often a hint that the machine is shutting down. Unlike Squirrel.Mac which always waits until app close to apply the update, Squirrel.Windows can fully apply an update while the app is running. So on Windows, it makes sense to try to wait until the app is idle but still running, then fully download and apply updates.

Squirrel.Windows does updates out-of-process, so closing the app actually shouldn't cancel updates, though node.js/Electron may be explicitly or inadvertently closing all spawned processes from the main app. You may be able to get around this via directly calling spawn on update.exe and passing {detached: true} as an option

@sergiou87
Copy link

That makes total sense, thanks!! In this case, we're trying to mitigate the issue on Windows (we haven't had reports of anything similar happening on macOS), so I hope the current approach is good enough for that, as the update is being applied with the app running thanks to those checks every 4h. This, however, is interesting:

Squirrel.Windows does updates out-of-process, so closing the app actually shouldn't cancel updates, though node.js/Electron may be explicitly or inadvertently closing all spawned processes from the main app. You may be able to get around this via directly calling spawn on update.exe and passing {detached: true} as an option

because I've definitely seen the update process being interrupted when I close the app (and therefore leaving the temp files and sometimes even a corrupted installation 😕 )

After chatting with @niik about this, we're gonna try this out without explicitly invoking update.exe, since that could break if Electron/Squirrel.Windows changed their implementation. We hope this is not a very common problem and maybe the simple changes from my PR are enough to mitigate the issue almost entirely. We'll see!! 🤞🤞🤞 Another thing we'll consider is using this Windows API to prevent Windows from shutting down (politely 😆 ) while the app is updating.

In any case, thanks a lot for your feedback, it super helpful! ❤️

@anaisbetts
Copy link
Contributor

that could break if Electron/Squirrel.Windows changed their implementation

This will never break, the location and name of Update.exe is Guaranteed™️ for compatibility purposes

@jaywalker291
Copy link

Not happy to find 35 gig of "temp" files on my work laptop today.

Can I manually delete everything in this folder? I have no idea what "squirrel" is, but I do not like it one bit. What a joke!

sq

@sergiou87
Copy link

@jaywalker291 yes, you can delete it manually. Those are just installer files. Squirrel is what some apps use to auto-update, and sometimes the process can fail and leave those temp files behind.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests