Closing the game during update at just the right time will not open again

[SonoSooS]

Type

Other

Bug description

If I close the game during an update at just the right time (discovered by accident), then clicking the shortcut icon on the desktop will not launch the game anymore. Checked in Process Explorer, the executable closes so fast that it doesn't even show up most of the time. Trying to run %LOCALAPPDATA%\osulazer\osu!.exe indeed just closes. Running the previous version's exe directly works just fine however.

Seems like it's an issue with the Squirrel stub itself, as it fails even if the new version's folder just exists, empty or not. Deleting the new version's folder "fixes" the issue, and allows the old version to open again, so it can download the new version.

Screenshots or videos

https://www.youtube.com/watch?v=pLpDbaZWCA4

Version

2022.509.0

Logs

runtime.log
updater.log
performance.log
database.log
performance-draw.log
performance-update.log

[smoogipoo]

@caesay may be interested in this.

Could you also check to see if there's any exceptions in event viewer relating to the stub? The runtime.log there seems to be from the prior/working version.

[SonoSooS]

Yes, the runtime.log is from the second time I triggered the bug, like shown on the video. It was basically the same method as on the video, except for the video I re-recorded it a few times, as the timing matters (there is like a 1-3s window for this bug to be triggered on my machine, and I managed to miss it a few times by being too early or too late).

There is sadly absolutely no useful information in Event Viewer besides "Failed to create CoreCLR".

One thing I tried though, is deleting everything but osu!.exe and osu!.dll, and running the exe directly (from the app folder) has asked me to download .NET Core 6.0.2, even though I already have it installed???

[smoogipoo]

Right, so that would be expected. I believe what's going on there is one of the dependencies are missing (in your screenshot it's coreclr itself), and the apphost (osu!.exe) is trying to open that osu!.deps.json file to figure out how/where to load it from as a shared library.
Neither of our .nupkg nor .exe contains osu!.deps.json so it can't do that, but that's not critical because the app is self-contained and requires all the files alongside it to work. At the end of the day the issue is that one of the dependencies is missing.

I peeped slightly into Squirrel's source and it seems it extracts directly into the app-{version} directory, which could leave it in a damaged state if the app is quit/power is lost. One way it could handle this is if it instead extracted to say, a _app.{version} directory and then renamed it inplace to the final name after the extraction completes.

@caesay Curious to hear your thoughts on this / whether it's something you'd consider doing / whether there's another way around this. Would be happy to make an issue on Squirrel for it if you deem reasonable.

---

As an aside and completely unrelated: It's slightly concerning to me that all .json files are missing from the release which includes a potential osu!.runtimeconfig.json file that can be used to e.g. tweak the GC in the future. This would come down to the includes defined in the .nuspec which is... Slightly outdated and only works because nuget.exe is used for packing 😅

[peppy]

The missing osu!.deps.json has come up in almost every event viewer log I've seen provided to date. Always considered it to be a red herring, but if we can fix it I'd love to see it fixed.

[caesay]

Not going to comment on any specific launch errors, because if there are missing dll's or missing json files (still required in self contained deployments!) in the app dir you can get all sorts of weird errors from dotnet. The stub is not to blame, really - it just tries to launch the latest version and exit.

On the Squirrel architecture point, old Squirrel downloads new versions to app-{ver} and as soon as a new directory exists (even if empty), everything will start treating that as the current version. There are lots of issues related to the app binary moving around. clowd/Clowd.Squirrel#24 has a non-exhaustive list, there are many other problems. One particular problem with this approach is that you have an update process that is not atomic - there is no "swap" of the old version to the new version. You could have app-1.0.0 running, you could download an update for app-1.0.1, click a shortcut, it would launch the latest version (1.0.1), and now you have both 1.0.0 and 1.0.1 running at the same time! not great.

Clowd.Squirrel 3.0.0 (currently unreleased, eta 2 weeks on first prerelease) has a totally new approach. Firstly, the app will be run from a current directory instead of an app-{ver} directory. You can still download releases in the background, but it will now be necessary to restart the app (using UpdateManager.RestartApp, or just closing and re-launching the shortcut or a stub) for the new version to be placed into the current directory. This means the app will always run from the same location, and you'll only ever have binaries from one version at a time running.

If the shortcut is clicked and for any reason we can't replace the current dir on app start (eg. it is still running) we will just fail to update silently and continue running the old version. Calling UpdateManager.RestartApp is more aggressive and will try to kill any running processes in the dir to force the update. Calling UpdateManager.UpdateApp will also be a no-op if there is a local pending version ready to be applied. (eg. call RestartApp to finish the update)

This new approach solves a lot of problems; and it also opens up MacOS support since essentially what we are doing on Mac is just replacing a .app dir in place, instead of replacing a current dir in place on windows.

There are two problems caused by the new approach that I have not quite solved yet (not an issue on Mac, only on Windows):

1. Pinned shortcuts will actually point to %localappdir%\yourapp\current\yourapp.exe instead of to the stub executable one folder up. This means if a pinned shortcut is executed, we don't have any chance to apply a pending update to the current dir before the app launches.
2. We still have no way to 'detect' that an update was left like half extracted and is broken, so we'll still swap out current for a broken version the next time a shortcut or stub gets executed.

For 1. there are a couple of options. Squirrel.HandleEvents could run Update.exe in watcher mode, essentially just waiting quietly for the app to exit. Every time the app exits, Update.exe will try to apply any current dir update and then quit. Another option could be that Squirrel.HandleEvents could just detect that the app is being run and there is a pending update and trigger a restart (but it might be a little more fiddly to try and detect that there are no other app exe's running.. and the update can actually be applied so we should restart)

For 2. there are also lots of options, perhaps the simplest thing could be to write the Squirrel version metadata to the directory as the last operation when downloading an update. If this metadata file is ever missing in a pending update folder, we just assume it's a broken version and throw it away. A more complex approach perhaps could be to try and detect that the app is broken, by re-checking the files in the directory against the downloaded .nupkg before applying a current dir update. (related to clowd/Clowd.Squirrel#55). Yet another more complex approach could be to run the app and see if it exits immediately with an error code. If so, we could attempt to roll back to the previous version still on disk.

Certainly willing to hear any feedback about 1. and 2. and will consider it as I get closer to a v3.0.0 pre-release.

[caesay]

On another note, I would highly advise that you no longer package your own nupkg via nuget.exe - and instead migrate to using the new pack command. It takes a folder of application files and spits out a Squirrel release in one step - and although I have no plans to deprecate the releasify command at this moment, it could happen in the future - and in either case, using pack means you'll always get the most optimal result as we will always do all the right things when creating the nupkg.

P.S. the Mac version of the Squirrel bundler does not allow you to give it a pre-assembled nupkg. it expects either a folder of application files to bundle, or it expects a pre-assembled .app folder to turn into a release.

[peppy]

On another note, I would highly advise that you no longer package your own nupkg via nuget.exe

Tracking this at ppy/osu-deploy#125.

[caesay]

In current xplat branch Squirrel now extracts new versions into a temporary directory and moves it into place when completed to minimize the risk of a half-extracted update being used.