From 668463f687a85eaafd6a0cf1ab92db81da649cf7 Mon Sep 17 00:00:00 2001 From: Frog Date: Tue, 2 Jun 2026 00:31:35 -0700 Subject: [PATCH] Improve Potential Async Void Crash Risks Added try catch for async handling across the application to reduce crash risk from async void methods. Added proper error handling so exceptions are caught instead of crashing the app. --- CreamInstaller/Forms/SelectForm.cs | 61 ++++-- CreamInstaller/Forms/UpdateForm.cs | 333 ++++++++++++++++------------- CreamInstaller/Program.cs | 66 +++++- 3 files changed, 282 insertions(+), 178 deletions(-) diff --git a/CreamInstaller/Forms/SelectForm.cs b/CreamInstaller/Forms/SelectForm.cs index 9ad5f87..4e5c925 100644 --- a/CreamInstaller/Forms/SelectForm.cs +++ b/CreamInstaller/Forms/SelectForm.cs @@ -556,27 +556,29 @@ internal sealed partial class SelectForm : CustomForm private async void OnLoad(bool forceScan = false, bool forceProvideChoices = false) { - Program.Canceled = false; - blockedGamesCheckBox.Enabled = false; - blockProtectedHelpButton.Enabled = false; - useSmokeAPICheckBox.Enabled = false; - useSmokeAPIHelpButton.Enabled = false; - cancelButton.Enabled = true; - scanButton.Enabled = false; - noneFoundLabel.Visible = false; - allCheckBox.Enabled = false; - proxyAllCheckBox.Enabled = false; - installButton.Enabled = false; - uninstallButton.Enabled = installButton.Enabled; - selectionTreeView.Enabled = false; - saveButton.Enabled = false; - loadButton.Enabled = false; - resetButton.Enabled = false; - progressLabel.Text = "Waiting for user to select which programs/games to scan . . ."; - ShowProgressBar(); - await ProgramData.Setup(this); - ProgramData.ClearLog(); - ProgramData.Log($"[Scan] CreamInstaller {Program.Version} — scan started at {DateTime.UtcNow:yyyy-MM-dd HH:mm:ss} UTC"); + try + { + Program.Canceled = false; + blockedGamesCheckBox.Enabled = false; + blockProtectedHelpButton.Enabled = false; + useSmokeAPICheckBox.Enabled = false; + useSmokeAPIHelpButton.Enabled = false; + cancelButton.Enabled = true; + scanButton.Enabled = false; + noneFoundLabel.Visible = false; + allCheckBox.Enabled = false; + proxyAllCheckBox.Enabled = false; + installButton.Enabled = false; + uninstallButton.Enabled = installButton.Enabled; + selectionTreeView.Enabled = false; + saveButton.Enabled = false; + loadButton.Enabled = false; + resetButton.Enabled = false; + progressLabel.Text = "Waiting for user to select which programs/games to scan . . ."; + ShowProgressBar(); + await ProgramData.Setup(this); + ProgramData.ClearLog(); + ProgramData.Log($"[Scan] CreamInstaller {Program.Version} — scan started at {DateTime.UtcNow:yyyy-MM-dd HH:mm:ss} UTC"); bool scan = forceScan; if (!scan && (programsToScan is null || programsToScan.Count < 1 || forceProvideChoices)) { @@ -712,6 +714,23 @@ internal sealed partial class SelectForm : CustomForm blockProtectedHelpButton.Enabled = true; useSmokeAPICheckBox.Enabled = true; useSmokeAPIHelpButton.Enabled = true; + } + catch (Exception ex) + { + // Handle exceptions in async void to prevent unobserved exceptions +#if DEBUG + System.Diagnostics.Debug.WriteLine($"OnLoad exception: {ex.Message}"); +#endif + // Show error and clean up + ex.HandleException(this); + HideProgressBar(); + cancelButton.Enabled = false; + scanButton.Enabled = true; + blockedGamesCheckBox.Enabled = true; + blockProtectedHelpButton.Enabled = true; + useSmokeAPICheckBox.Enabled = true; + useSmokeAPIHelpButton.Enabled = true; + } } private void OnTreeViewNodeCheckedChanged(object sender, TreeViewEventArgs e) diff --git a/CreamInstaller/Forms/UpdateForm.cs b/CreamInstaller/Forms/UpdateForm.cs index d9809b0..6a474e6 100644 --- a/CreamInstaller/Forms/UpdateForm.cs +++ b/CreamInstaller/Forms/UpdateForm.cs @@ -45,62 +45,76 @@ internal sealed partial class UpdateForm : CustomForm private async void OnLoad() { - progressBar.Visible = false; - ignoreButton.Visible = true; - updateButton.Text = "Update"; - updateButton.Click -= OnUpdateCancel; - progressLabel.Text = "Checking for updates . . ."; - changelogTreeView.Visible = false; - changelogTreeView.Location = progressLabel.Location with + try { - Y = progressLabel.Location.Y + progressLabel.Size.Height + 13 - }; - Refresh(); -#if !DEBUG - Version currentVersion = new(Program.Version); -#endif - List releases = null; - string response = - await HttpClientManager.EnsureGet( - $"https://api.github.com/repos/{Program.RepositoryOwner}/{Program.RepositoryName}/releases"); - if (response is not null) - releases = JsonConvert.DeserializeObject>(response) - ?.Where(release => !release.Draft && !release.Prerelease && release.Asset is not null).ToList(); - latestRelease = releases?.FirstOrDefault(); -#if DEBUG - if (latestRelease?.Version is not { } latestVersion) -#else - if (latestRelease?.Version is not { } latestVersion || latestVersion <= currentVersion) -#endif - StartProgram(); - else - { - progressLabel.Text = $"An update is available: v{latestVersion}"; - ignoreButton.Enabled = true; - updateButton.Enabled = true; - updateButton.Click += OnUpdate; - changelogTreeView.Visible = true; - foreach (ProgramRelease release in releases) + progressBar.Visible = false; + ignoreButton.Visible = true; + updateButton.Text = "Update"; + updateButton.Click -= OnUpdateCancel; + progressLabel.Text = "Checking for updates . . ."; + changelogTreeView.Visible = false; + changelogTreeView.Location = progressLabel.Location with { + Y = progressLabel.Location.Y + progressLabel.Size.Height + 13 + }; + Refresh(); #if !DEBUG - if (release.Version <= currentVersion) - continue; + Version currentVersion = new(Program.Version); #endif - TreeNode root = new(release.Name) { Name = release.Name }; - changelogTreeView.Nodes.Add(root); - if (changelogTreeView.Nodes.Count > 0) - changelogTreeView.Nodes[0].EnsureVisible(); - foreach (string change in release.Changes) - Invoke(delegate - { - TreeNode changeNode = new() { Text = change }; - root.Nodes.Add(changeNode); - root.Expand(); - if (changelogTreeView.Nodes.Count > 0) - changelogTreeView.Nodes[0].EnsureVisible(); - }); + List releases = null; + string response = + await HttpClientManager.EnsureGet( + $"https://api.github.com/repos/{Program.RepositoryOwner}/{Program.RepositoryName}/releases"); + if (response is not null) + releases = JsonConvert.DeserializeObject>(response) + ?.Where(release => !release.Draft && !release.Prerelease && release.Asset is not null).ToList(); + latestRelease = releases?.FirstOrDefault(); +#if DEBUG + if (latestRelease?.Version is not { } latestVersion) +#else + if (latestRelease?.Version is not { } latestVersion || latestVersion <= currentVersion) +#endif + StartProgram(); + else + { + progressLabel.Text = $"An update is available: v{latestVersion}"; + ignoreButton.Enabled = true; + updateButton.Enabled = true; + updateButton.Click += OnUpdate; + changelogTreeView.Visible = true; + foreach (ProgramRelease release in releases) + { +#if !DEBUG + if (release.Version <= currentVersion) + continue; +#endif + TreeNode root = new(release.Name) { Name = release.Name }; + changelogTreeView.Nodes.Add(root); + if (changelogTreeView.Nodes.Count > 0) + changelogTreeView.Nodes[0].EnsureVisible(); + foreach (string change in release.Changes) + Invoke(delegate + { + TreeNode changeNode = new() { Text = change }; + root.Nodes.Add(changeNode); + root.Expand(); + if (changelogTreeView.Nodes.Count > 0) + changelogTreeView.Nodes[0].EnsureVisible(); + }); + } } } + catch (Exception ex) + { + // Handle exceptions in async void to prevent unobserved exceptions +#if DEBUG + System.Diagnostics.Debug.WriteLine($"OnLoad exception: {ex.Message}"); + ex.HandleFatalException(); +#else + // In release, try to continue gracefully + StartProgram(); +#endif + } } private void OnLoad(object sender, EventArgs _) @@ -123,122 +137,135 @@ internal sealed partial class UpdateForm : CustomForm private async void OnUpdate(object sender, EventArgs e) { - progressBar.Value = 0; - progressBar.Visible = true; - ignoreButton.Visible = false; - updateButton.Text = "Cancel"; - updateButton.Click -= OnUpdate; - updateButton.Click += OnUpdateCancel; - changelogTreeView.Location = - progressBar.Location with { Y = progressBar.Location.Y + progressBar.Size.Height + 6 }; - Refresh(); - Progress progress = new(); - IProgress iProgress = progress; - progress.ProgressChanged += delegate(object _, int _progress) - { - progressLabel.Text = $"Updating . . . {_progress}%"; - progressBar.Value = _progress; - }; - progressLabel.Text = "Updating . . . "; - cancellation = new(); - bool success = true; - PackagePath.DeleteFile(true); - await using FileStream update = PackagePath.CreateFile(true); - bool retry = true; try { - if (cancellation is null || Program.Canceled) - throw new TaskCanceledException(); - using HttpResponseMessage response = await HttpClientManager.HttpClient.GetAsync( - latestRelease.Asset.BrowserDownloadUrl, - HttpCompletionOption.ResponseHeadersRead, cancellation.Token); - _ = response.EnsureSuccessStatusCode(); - if (cancellation is null || Program.Canceled) - throw new TaskCanceledException(); - await using Stream download = await response.Content.ReadAsStreamAsync(cancellation.Token); - double bytes = latestRelease.Asset.Size; - byte[] buffer = new byte[16384]; - long bytesRead = 0; - int newBytes; - while (cancellation is not null && !Program.Canceled - && (newBytes = await download.ReadAsync(buffer.AsMemory(0, buffer.Length), - cancellation.Token)) != 0) + progressBar.Value = 0; + progressBar.Visible = true; + ignoreButton.Visible = false; + updateButton.Text = "Cancel"; + updateButton.Click -= OnUpdate; + updateButton.Click += OnUpdateCancel; + changelogTreeView.Location = + progressBar.Location with { Y = progressBar.Location.Y + progressBar.Size.Height + 6 }; + Refresh(); + Progress progress = new(); + IProgress iProgress = progress; + progress.ProgressChanged += delegate(object _, int _progress) + { + progressLabel.Text = $"Updating . . . {_progress}%"; + progressBar.Value = _progress; + }; + progressLabel.Text = "Updating . . . "; + cancellation = new(); + bool success = true; + PackagePath.DeleteFile(true); + await using FileStream update = PackagePath.CreateFile(true); + bool retry = true; + try { if (cancellation is null || Program.Canceled) throw new TaskCanceledException(); - await update.WriteAsync(buffer.AsMemory(0, newBytes), cancellation.Token); - bytesRead += newBytes; - int report = (int)(bytesRead / bytes * 100); - if (report <= progressBar.Value) - continue; - iProgress.Report(report); + using HttpResponseMessage response = await HttpClientManager.HttpClient.GetAsync( + latestRelease.Asset.BrowserDownloadUrl, + HttpCompletionOption.ResponseHeadersRead, cancellation.Token); + _ = response.EnsureSuccessStatusCode(); + if (cancellation is null || Program.Canceled) + throw new TaskCanceledException(); + await using Stream download = await response.Content.ReadAsStreamAsync(cancellation.Token); + double bytes = latestRelease.Asset.Size; + byte[] buffer = new byte[16384]; + long bytesRead = 0; + int newBytes; + while (cancellation is not null && !Program.Canceled + && (newBytes = await download.ReadAsync(buffer.AsMemory(0, buffer.Length), + cancellation.Token)) != 0) + { + if (cancellation is null || Program.Canceled) + throw new TaskCanceledException(); + await update.WriteAsync(buffer.AsMemory(0, newBytes), cancellation.Token); + bytesRead += newBytes; + int report = (int)(bytesRead / bytes * 100); + if (report <= progressBar.Value) + continue; + iProgress.Report(report); + } + + iProgress.Report((int)(bytesRead / bytes * 100)); + if (cancellation is null || Program.Canceled) + throw new TaskCanceledException(); + } + catch (TaskCanceledException) + { + success = false; + } + catch (Exception ex) + { + retry = ex.HandleException(this, Program.Name + " encountered an exception while updating"); + success = false; } - iProgress.Report((int)(bytesRead / bytes * 100)); - if (cancellation is null || Program.Canceled) - throw new TaskCanceledException(); - } - catch (TaskCanceledException) - { - success = false; + cancellation?.Dispose(); + cancellation = null; + await update.DisposeAsync(); + bool canContinue = success && !Program.Canceled; + if (canContinue) + updateButton.Enabled = false; + ExecutablePath.DeleteFile(canContinue); + if (canContinue) + await Task.Run(() => PackagePath.ExtractZip(ProgramData.DirectoryPath, true, this)); + PackagePath.DeleteFile(canContinue); + if (canContinue) + { + string path = Program.CurrentProcessFilePath; + string directory = Path.GetDirectoryName(path); + string file = Path.GetFileName(path); + StringBuilder commands = new(); + _ = commands.AppendLine(CultureInfo.InvariantCulture, $"chcp 65001"); + _ = commands.AppendLine(CultureInfo.InvariantCulture, $":LOOP"); + _ = commands.AppendLine(CultureInfo.InvariantCulture, $"TASKKILL /F /T /PID {Program.CurrentProcessId}"); + _ = commands.AppendLine(CultureInfo.InvariantCulture, $"TASKLIST | FIND \" {Program.CurrentProcessId} \""); + _ = commands.AppendLine(CultureInfo.InvariantCulture, $"IF NOT ERRORLEVEL 1 ("); + _ = commands.AppendLine(CultureInfo.InvariantCulture, $" TIMEOUT /T 1"); + _ = commands.AppendLine(CultureInfo.InvariantCulture, $" GOTO LOOP"); + _ = commands.AppendLine(CultureInfo.InvariantCulture, $")"); + _ = commands.AppendLine(CultureInfo.InvariantCulture, $"MOVE /Y \"{ExecutablePath}\" \"{path}\""); + _ = commands.AppendLine(CultureInfo.InvariantCulture, $"START \"\" /D \"{directory}\" \"{file}\""); +#if DEBUG + _ = commands.AppendLine(CultureInfo.InvariantCulture, $"PAUSE"); +#endif + _ = commands.AppendLine(CultureInfo.InvariantCulture, $"EXIT"); + UpdaterPath.WriteFile(commands.ToString(), true, this, Encoding.Default); + Process process = new(); + ProcessStartInfo startInfo = new() + { + WorkingDirectory = ProgramData.DirectoryPath, FileName = "cmd.exe", + Arguments = $"/C START \"UPDATER\" /B {Path.GetFileName(UpdaterPath)}", +#if DEBUG + CreateNoWindow = false +#else + CreateNoWindow = true +#endif + }; + process.StartInfo = startInfo; + _ = process.Start(); + return; + } + + if (!retry) + StartProgram(); + else + OnLoad(); } catch (Exception ex) { - retry = ex.HandleException(this, Program.Name + " encountered an exception while updating"); - success = false; - } - - cancellation?.Dispose(); - cancellation = null; - await update.DisposeAsync(); - bool canContinue = success && !Program.Canceled; - if (canContinue) - updateButton.Enabled = false; - ExecutablePath.DeleteFile(canContinue); - if (canContinue) - await Task.Run(() => PackagePath.ExtractZip(ProgramData.DirectoryPath, true, this)); - PackagePath.DeleteFile(canContinue); - if (canContinue) - { - string path = Program.CurrentProcessFilePath; - string directory = Path.GetDirectoryName(path); - string file = Path.GetFileName(path); - StringBuilder commands = new(); - _ = commands.AppendLine(CultureInfo.InvariantCulture, $"chcp 65001"); - _ = commands.AppendLine(CultureInfo.InvariantCulture, $":LOOP"); - _ = commands.AppendLine(CultureInfo.InvariantCulture, $"TASKKILL /F /T /PID {Program.CurrentProcessId}"); - _ = commands.AppendLine(CultureInfo.InvariantCulture, $"TASKLIST | FIND \" {Program.CurrentProcessId} \""); - _ = commands.AppendLine(CultureInfo.InvariantCulture, $"IF NOT ERRORLEVEL 1 ("); - _ = commands.AppendLine(CultureInfo.InvariantCulture, $" TIMEOUT /T 1"); - _ = commands.AppendLine(CultureInfo.InvariantCulture, $" GOTO LOOP"); - _ = commands.AppendLine(CultureInfo.InvariantCulture, $")"); - _ = commands.AppendLine(CultureInfo.InvariantCulture, $"MOVE /Y \"{ExecutablePath}\" \"{path}\""); - _ = commands.AppendLine(CultureInfo.InvariantCulture, $"START \"\" /D \"{directory}\" \"{file}\""); + // Handle exceptions in async void event handler to prevent unobserved exceptions #if DEBUG - _ = commands.AppendLine(CultureInfo.InvariantCulture, $"PAUSE"); + System.Diagnostics.Debug.WriteLine($"OnUpdate exception: {ex.Message}"); #endif - _ = commands.AppendLine(CultureInfo.InvariantCulture, $"EXIT"); - UpdaterPath.WriteFile(commands.ToString(), true, this, Encoding.Default); - Process process = new(); - ProcessStartInfo startInfo = new() - { - WorkingDirectory = ProgramData.DirectoryPath, FileName = "cmd.exe", - Arguments = $"/C START \"UPDATER\" /B {Path.GetFileName(UpdaterPath)}", -#if DEBUG - CreateNoWindow = false -#else - CreateNoWindow = true -#endif - }; - process.StartInfo = startInfo; - _ = process.Start(); - return; - } - - if (!retry) + // Show error to user + ex.HandleException(this, Program.Name + " encountered an unexpected error during update"); StartProgram(); - else - OnLoad(); + } } private void OnUpdateCancel(object sender, EventArgs e) diff --git a/CreamInstaller/Program.cs b/CreamInstaller/Program.cs index ef3f924..f93303c 100644 --- a/CreamInstaller/Program.cs +++ b/CreamInstaller/Program.cs @@ -2,6 +2,7 @@ using System; using System.Diagnostics; using System.Linq; using System.Threading; +using System.Threading.Tasks; using System.Windows.Forms; using CreamInstaller.Forms; using CreamInstaller.Platforms.Steam; @@ -91,15 +92,72 @@ internal static class Program internal static bool Canceled; - internal static async void Cleanup(bool cancel = true) + /// + /// Initiates application cleanup asynchronously. Use this when you can await the result. + /// + /// Whether to set the Canceled flag + /// Task that completes when cleanup is finished + internal static async Task CleanupAsync(bool cancel = true) { - Canceled = cancel; + if (cancel) + Canceled = true; await SteamCMD.Cleanup(); } + /// + /// Synchronous cleanup wrapper for event handlers and other synchronous contexts. + /// Initiates cleanup without blocking but does not wait for completion. + /// + /// Whether to set the Canceled flag + internal static void Cleanup(bool cancel = true) + { + if (cancel) + Canceled = true; + + // Fire and forget - don't block synchronous callers + // Any exceptions will be logged but won't crash the app + _ = Task.Run(async () => + { + try + { + await SteamCMD.Cleanup(); + } + catch (Exception ex) + { +#if DEBUG + System.Diagnostics.Debug.WriteLine($"Cleanup failed: {ex.Message}"); +#endif + // Swallow exceptions during fire-and-forget cleanup + } + }); + } + private static void OnApplicationExit(object s, EventArgs e) { - Cleanup(); - HttpClientManager.Dispose(); + Canceled = true; + + // For application exit, we should try to wait briefly for cleanup + try + { + Task cleanupTask = SteamCMD.Cleanup(); + // Wait up to 5 seconds for graceful cleanup + if (!cleanupTask.Wait(TimeSpan.FromSeconds(5))) + { +#if DEBUG + System.Diagnostics.Debug.WriteLine("Cleanup timed out during application exit"); +#endif + } + } + catch (Exception ex) + { +#if DEBUG + System.Diagnostics.Debug.WriteLine($"Cleanup exception during exit: {ex.Message}"); +#endif + // Ignore exceptions during shutdown + } + finally + { + HttpClientManager.Dispose(); + } } }