Possible issues with concurrent installations

[bzuillsmith]

In Squirrel.Windows there is a bug that multiple deployments running at the same time on login can overwrite each other's files in the extraction process before Updater is called to install each. I am thinking about switching to Clowd.Squirrel but wanted to see if the same problem could occur in this program. I quick look makes me think the same issue may exist. Two programs deploying on login could overwrite each other's files as they both write to %temp%\squirrel. See updaterPath and get_temp_file_path() Am I reading this correctly (not a C++ guy)? Or are there steps to ensure they extract to different places that I'm not quite seeing.

[caesay]

I had seen the conversation at Squirrel#1138 yesterday and was thinking about this same problem.

The only part of Squirrel that I can think of which may impact another install is the temp folder selection. It has been improved in this fork (old squirrel will leave temp folders laying around while this library tries harder to clean up) but I think it will suffer the same issue at the moment.

Essentially both old squirrel and this fork will iterate trying to find the next available free temp directory (eg. temp1, temp2, temp3). If two installs pick the same temp folder name at the same time that can be problematic. See the code here. The C++ code you found is not a problem, that is only used to extract Update.exe and GetTempFileName guarantees it will be unique.

This is related to #70. We need to improve the mutex/locking, so that two installers installing the same package id will not run at the same time (the one failing to grab the mutex will fail and exit). We can also use a hash of the package id in the temp directory names so it is not possible for installers of different applications to select the same temp directory.

[bzuillsmith]

Hmm, but does the server deployment msi/exe actually use the tempA, tempB, etc folders when extracting the updater, RELEASES and nupkg? squirrel windows does not.

[caesay]

Both original squirrel and this fork both use the same strategy for selecting a temp folder for the install. Original squirrel will count up alphanumerically (eg. tempa, tempb ... tempz) and this version just numerically (eg. temp.1, temp.2 ... temp.10).

Both the original and this fork creates temp folder(s) during any installation and any update.

I will look to fix in this fork by making the temp folder creation logic use some key which is unique to your application package ID, therefore preventing temp directory collisions during install/update.

[bzuillsmith]

Squirrel.Windows appears to use tempa tempb when running a typical install. But what I mean is that the "Deployment Tool" does not. I have a verified this in Squirrel.Windows already. Perhaps there was an intent to use the sub folders in all cases, but then this was missed. Updater, RELEASES, and nupkg are extracted directly to SquirrelTemp, not the tempa/b/c subfolders.

[caesay]

The Deployment Tool absolutely uses temp directories in the same way. The MSI deployment tool just extracts the regular Setup.exe to %programfiles% and runs it every time any user logs on. This probably exacerbates the issue significantly, since if you have multiple Squirrel deployment tools installed, it will be running multiple Setup.exe's at the same time on every logon.

[caesay]

It's possible that the old Squirrel Setup.exe does also extract some files directly to SquirrelTemp. I don't think so, but I'm not 100% confident. However, that's not done here in this fork. The MSI runs Setup.exe, Setup.exe extracts only Update.exe to %temp% at a unique file path, and then Update.exe does the rest using the typical temp.1, temp.2 folders.

[bzuillsmith]

Got it. Good to know Clowd.Squirrel does things a little better. Sounds like there is still some potential for problems, but much less.

To see the behavior in Squirrel.Windows -- this line and this line set the target directory for extracting files from Setup.exe and this line sets it as the base directory for the extracted files. I watched the directory as the install ran and saw the files I mentioned above placed in the directory temporarily.

Feel free to close unless you feel there are things to be addressed in Clowd.Squirrel related to this

[caesay]

In the specific parts of code you pointed out - those are not an issue here. I will leave this issue open as I still think it's a good idea to address the incrementing temp folder creation - there is still potential for collisions there (maybe less likely than the issue you pointed out) but it will be trivial to fix.