JosefNemec / PlayniteExtensions

Extensions for Playnite game launcher and manager.
MIT License
174 stars 41 forks source link

Steam: Fix freezes during Uninstall #416

Closed darklinkpower closed 2 weeks ago

darklinkpower commented 2 weeks ago

Fixes:

Issue doesn't occur in Install controller because the Watcher is already running in a Task.

Before:

https://github.com/user-attachments/assets/f4cb244b-1a67-4c78-bd6e-1121322f8486

After:

https://github.com/user-attachments/assets/32bce657-2e0c-4132-ac9b-ff35adefcc82

darklinkpower commented 2 weeks ago

Additionally, I'd like to point that currently there's an issue in the Steam and other plugins, although maybe it's not a big issue in itself. Currently the CancellationTokenSource is not disposed of: https://github.com/JosefNemec/PlayniteExtensions/blob/7c471edfeea4ed1384f97c14ad76c48988453248/source/Libraries/SteamLibrary/SteamGameController.cs#L23-L26

Disposing during the Controllers Dispose() method is not possible because it will throw an exception in the running Install/Uninstall Watchers while checking its token. As an alternative proposal, perhaps a simple isCancelled boolean field could be used instead of a token. In my opinion a simpler approach without drawbacks, more so because the token is not being used for any special anyway, other than checking a simple boolean value too.

    public class SteamUninstallController : UninstallController
    {
        private bool isCancelled = false;

        public SteamUninstallController(Game game) : base(game)
        {
            Name = "Uninstall using Steam";
        }

        public override void Dispose()
        {
            isCancelled = true;
        }

        public override void Uninstall(UninstallActionArgs args)
        {
            if (!Steam.IsInstalled)
            {
                throw new Exception("Steam installation not found.");
            }

            var gameId = Game.ToSteamGameID();
            if (gameId.IsMod)
            {
                throw new NotSupportedException("Uninstalling mods is not supported.");
            }
            else
            {
                ProcessStarter.StartUrl($"steam://uninstall/{gameId.AppID}");
                StartUninstallWatcher();
            }
        }

        public async void StartUninstallWatcher()
        {
            var id = Game.ToSteamGameID();
            isCancelled = false;

            await Task.Run(async () =>
            {
                while (true)
                {
                    if (isCancelled)
                    {
                        return;
                    }

                    var installed = SteamLibrary.GetInstalledGames(false);
                    if (!installed.ContainsKey(id))
                    {
                        InvokeOnUninstalled(new GameUninstalledEventArgs());
                        return;
                    }

                    await Task.Delay(5_000);
                }
            });
        }
    }
JosefNemec commented 2 weeks ago

The reason why CancellationTokenSource is used here instead of simple bool value is because it's thread safe. Probably doesn't matter in this simple use case, but generally it's the reason why it's being used instead of simple bool value to track action cancellations.

JosefNemec commented 2 weeks ago

Btw the proper fix for that would be setting cancel on cancellation token, waiting for tracking task to finish, then dispose.

darklinkpower commented 2 weeks ago

Got it, right now I can't take look since I'll be busy with the addons I develop but I'll try to take a look when I can to fix the Dispose issue in all the plugins here as part of another PR.