Fix cancelling a restart for update still quitting the game after one minutes

[peppy]

Closes #36816.

• Self review / testing.

[bdach]

Seems mostly ok if nothing breaks in testing

[smoogipoo]

I really don't like this Attempt-Confirm-Cancel song and dance. I have no suggestions at this point but I hope these few comments illustrate my uncertainty with this change.

[smoogipoo]

This task is not guaranteed to have started by the time the game exits.

In other words, it is possible for the game to exit, no updates be applied, and the game not restarted for the user.

[peppy]

Yes, I've made this very clear in the inline comments (i hoped).

[smoogipoo]

But why? Why not just leave the running to the disposal?

[peppy]

Because of the thing you point out in #36950 (comment). This gives it a chance to run.

[smoogipoo]

This being basically only usable by one class, yet being public, makes me a little bit scared. Because if it's used by literally any other consumer (say, the database wants to do something on exiting) then the updater path falls over.

[peppy]

Do you prefer the version before 9531706 ?

[smoogipoo]

I think I do. The OsuGameDesktop usage (RestartAppWhenExited) is still weird imo, but maybe that's an unavoidable evil

That said, can we make things less cryptic while we're here?
RunOnExiting/OnExitingBegan -> RestartOnExitAction.

[smoogipoo]

Something I think is important, is that it seems all that WaitExitThenApplyUpdates() does is start the update exe, then internally sleep for 500ms (see: https://github.com/velopack/velopack/blob/ed8600eee530a38d2669444b03eb240e56bc5aa3/src/lib-csharp/UpdateExe.cs#L132-L141).

Given this, can the implementation be simplified to just:

diff --git a/osu.Desktop/OsuGameDesktop.cs b/osu.Desktop/OsuGameDesktop.cs
index 4ec57dedd1..40f70cdd92 100644
--- a/osu.Desktop/OsuGameDesktop.cs
+++ b/osu.Desktop/OsuGameDesktop.cs
@@ -20,7 +20,6 @@
 using osu.Game.Configuration;
 using osu.Game.IO;
 using osu.Game.IPC;
-using osu.Game.Online.Multiplayer;
 using osu.Game.Performance;
 using osu.Game.Utils;
 
@@ -123,7 +122,7 @@ protected override UpdateManager CreateUpdateManager()
 
         public override bool RestartAppWhenExited()
         {
-            RunOnExiting = () => Task.Run(() => Velopack.UpdateExe.Start(waitPid: (uint)Environment.ProcessId)).FireAndForget();
+            RestartOnExitAction = () => Velopack.UpdateExe.Start(waitPid: (uint)Environment.ProcessId);
             return true;
         }
 
diff --git a/osu.Desktop/Updater/VelopackUpdateManager.cs b/osu.Desktop/Updater/VelopackUpdateManager.cs
index a24cc52811..093eb60f10 100644
--- a/osu.Desktop/Updater/VelopackUpdateManager.cs
+++ b/osu.Desktop/Updater/VelopackUpdateManager.cs
@@ -8,7 +8,6 @@
 using osu.Framework.Logging;
 using osu.Framework.Threading;
 using osu.Game;
-using osu.Game.Online.Multiplayer;
 using osu.Game.Overlays;
 using osu.Game.Overlays.Notifications;
 using osu.Game.Screens.Play;
@@ -149,7 +148,7 @@ private void runOutsideOfGameplay(Action action, CancellationToken cancellationT
 
         private void restartToApplyUpdate(Velopack.UpdateManager updateManager, UpdateInfo update)
         {
-            game.RunOnExiting = () => Task.Run(() => updateManager.WaitExitThenApplyUpdates(update.TargetFullRelease)).FireAndForget();
+            game.RestartOnExitAction = () => updateManager.WaitExitThenApplyUpdates(update.TargetFullRelease);
             game.AttemptExit();
         }
 
diff --git a/osu.Game/OsuGame.cs b/osu.Game/OsuGame.cs
index 16adbca9ff..150d084a78 100644
--- a/osu.Game/OsuGame.cs
+++ b/osu.Game/OsuGame.cs
@@ -1756,12 +1756,6 @@ private void screenExited(IScreen lastScreen, IScreen newScreen)
         {
             ScreenChanged((OsuScreen)lastScreen, (OsuScreen)newScreen);
 
-            // Calling ConfirmExit() here allows it to run tasks in the background while the `IntroScreen` plays the outro on quit.
-            // Note that the user can early exit the outro sequence, so there is no guarantee for how long this task has.
-            // But let's assume that anything important is run inline in `ConfirmExit`.
-            if (lastScreen is MainMenu)
-                ConfirmExit();
-
             if (newScreen == null)
                 Exit();
         }
diff --git a/osu.Game/OsuGameBase.cs b/osu.Game/OsuGameBase.cs
index b2270b0e0c..e957753c8b 100644
--- a/osu.Game/OsuGameBase.cs
+++ b/osu.Game/OsuGameBase.cs
@@ -534,27 +534,17 @@ public virtual void AttemptExit()
         }
 
         /// <summary>
-        /// A special case action which is intended to restart or update the application on exit.
-        ///
-        /// It will be started as the main menu outro animation begins displaying, allowing a short window of
-        /// time which it can run tasks before the application quits.
+        /// An action that restarts the application after it has exited.
         /// </summary>
         [CanBeNull]
-        public Action RunOnExiting { private get; set; }
+        public Action RestartOnExitAction { private get; set; }
 
         /// <summary>
-        /// After an exit has been attempted via <see cref="AttemptExit"/>, this method can be called to
-        /// signal that the exit attempt has been cancelled.
+        /// Signals that the application should not be restarted after it is exited.
         /// </summary>
-        public void CancelExit() => RunOnExiting = null;
-
-        /// <summary>
-        /// When exiting has been confirmed, this should be called to run any pending exit action.
-        /// </summary>
-        protected void ConfirmExit()
+        public void CancelRestartOnExit()
         {
-            RunOnExiting?.Invoke();
-            RunOnExiting = null;
+            RestartOnExitAction = null;
         }
 
         /// <summary>
@@ -762,8 +752,6 @@ protected override void Dispose(bool isDisposing)
         {
             base.Dispose(isDisposing);
 
-            ConfirmExit();
-
             RulesetStore?.Dispose();
             LocalConfig?.Dispose();
 
@@ -773,6 +761,8 @@ protected override void Dispose(bool isDisposing)
 
             if (Host != null)
                 Host.ExceptionThrown -= onExceptionThrown;
+
+            RestartOnExitAction?.Invoke();
         }
 
         ControlPointInfo IBeatSyncProvider.ControlPoints => Beatmap.Value.BeatmapLoaded ? Beatmap.Value.Beatmap.ControlPointInfo : null;
diff --git a/osu.Game/Screens/Menu/MainMenu.cs b/osu.Game/Screens/Menu/MainMenu.cs
index 93f18849eb..32062409d0 100644
--- a/osu.Game/Screens/Menu/MainMenu.cs
+++ b/osu.Game/Screens/Menu/MainMenu.cs
@@ -429,7 +429,7 @@ public override bool OnExiting(ScreenExitEvent e)
                     }, () =>
                     {
                         holdToExitGameOverlay.Abort();
-                        Game.CancelExit();
+                        Game.CancelRestartOnExit();
                     }));
                 }
 

Or is it important for some reason that we run this in a background thread?

[peppy]

Given this, can the implementation be simplified to just:

I guess if we're okay with the dispose procedure sleeping for 500ms extra? It means everything will be delayed further. Maybe okay?

[smoogipoo]

It looks like more recent versions of velopack don't have that delay - the implementation of ApplyUpdatesAndRestart (I know this isn't the method we're using here, but it still calls WaitExitThenApplyUpdates()) event mentions "will exit your app immediately" and calls Exit(0).

So I consider the delay to be a temporary hindrance if anything?

[peppy]

Maybe 🤷

[peppy]

Have tested to work well for both updating and cancelling.

Note to self for testing locally:

• Delete /Users/dean/Library/Caches/velopack/osulazer
• in osu-deploy run dotnet run 1 2026.101.0 macOS arm64
• Use the version in releases not staging (copy to /Applications/).

[bdach]

I'm just going to cross fingers and hope that this works well because I am not willing to go through with installing dotnet 9 for velopack specifically on two machines just to test this.