From 9a134d43cb631cbda99ff45d5a7d3d1a3e157ae5 Mon Sep 17 00:00:00 2001 From: rmbakker88 Date: Sun, 26 Jul 2026 01:15:34 +0200 Subject: [PATCH] Fix hang when theme thumbnail cannot be generated GetThumbnailImage decodes images with System.Drawing, which throws ArgumentException or ExternalException for formats GDI+ cannot read. LoadThemes only caught OutOfMemoryException, so any other decode failure escaped the loop, faulted the background task started by LoadImportedThemes, and left the import dialog stuck on "Generating thumbnails, please wait..." with no error and nothing logged. The loadSemaphore was leaked at the same time, so later loads blocked for 60 seconds. Catch all exceptions per theme and log them, release the semaphore from a finally block (and only when it was acquired), and close the import dialog from a finally block so importing always completes. Co-Authored-By: Claude Opus 5 --- src/ThemeDialogUtils.cs | 131 ++++++++++++++++++++++++---------------- 1 file changed, 79 insertions(+), 52 deletions(-) diff --git a/src/ThemeDialogUtils.cs b/src/ThemeDialogUtils.cs index c259d7ac..19f1de76 100644 --- a/src/ThemeDialogUtils.cs +++ b/src/ThemeDialogUtils.cs @@ -52,66 +52,82 @@ internal static string[] GetDisplayNames() internal static void LoadThemes(List themes, ListView listView, ThemeLoadOpts opts) { - loadSemaphore.Wait(60000); - Size thumbnailSize = ThemeThumbLoader.GetThumbnailSize(listView); - ListViewItem focusedItem = null; + bool semaphoreAcquired = loadSemaphore.Wait(60000); - foreach (ThemeConfig theme in themes.ToList()) + try { - if (JsonConfig.settings.showInstalledOnly && !ThemeManager.IsThemeDownloaded(theme)) - { - continue; - } + Size thumbnailSize = ThemeThumbLoader.GetThumbnailSize(listView); + ListViewItem focusedItem = null; - try + foreach (ThemeConfig theme in themes.ToList()) { - using (Image thumbnailImage = ThemeThumbLoader.GetThumbnailImage(theme, thumbnailSize, true)) + if (JsonConfig.settings.showInstalledOnly && !ThemeManager.IsThemeDownloaded(theme)) { - listView.Invoke(new Action(() => - { - listView.LargeImageList.Images.Add(thumbnailImage); - string itemText = ThemeManager.GetThemeName(theme); - if (JsonConfig.settings.favoriteThemes != null && - JsonConfig.settings.favoriteThemes.Contains(theme.themeId)) - { - itemText = "★ " + itemText; - } - ListViewItem newItem = listView.Items.Add(itemText, - listView.LargeImageList.Images.Count - 1); - newItem.Tag = theme.themeId; + continue; + } - if (opts.activeTheme != null && opts.activeTheme == theme.themeId) - { - newItem.Font = new Font(newItem.Font, FontStyle.Bold); - } - if (opts.focusTheme == null || opts.focusTheme == theme.themeId) + try + { + using (Image thumbnailImage = ThemeThumbLoader.GetThumbnailImage(theme, thumbnailSize, true)) + { + listView.Invoke(new Action(() => { - focusedItem = newItem; - } - })); + listView.LargeImageList.Images.Add(thumbnailImage); + string itemText = ThemeManager.GetThemeName(theme); + if (JsonConfig.settings.favoriteThemes != null && + JsonConfig.settings.favoriteThemes.Contains(theme.themeId)) + { + itemText = "★ " + itemText; + } + ListViewItem newItem = listView.Items.Add(itemText, + listView.LargeImageList.Images.Count - 1); + newItem.Tag = theme.themeId; + + if (opts.activeTheme != null && opts.activeTheme == theme.themeId) + { + newItem.Font = new Font(newItem.Font, FontStyle.Bold); + } + if (opts.focusTheme == null || opts.focusTheme == theme.themeId) + { + focusedItem = newItem; + } + })); + } + } + catch (Exception exc) + { + // Image formats that cannot be decoded throw ArgumentException or ExternalException, so + // catching only OutOfMemoryException here used to let the exception escape and leave the + // import dialog waiting for thumbnails forever + LoggingHandler.LogMessage("Failed to generate thumbnail for '{0}' theme: {1}", theme.themeId, + exc); + listView.Invoke(new Action(() => + ThemeLoader.HandleError(new FailedToCreateThumbnail(theme.themeId)))); } } - catch (OutOfMemoryException) + + listView.Invoke(new Action(() => { - ThemeLoader.HandleError(new FailedToCreateThumbnail(theme.themeId)); - } - } + listView.Sort(); - listView.Invoke(new Action(() => - { - listView.Sort(); + if (focusedItem == null) + { + focusedItem = listView.Items[0]; + } + + focusedItem.Selected = true; + listView.EnsureVisible(focusedItem.Index); - if (focusedItem == null) + ThemeThumbLoader.CacheThumbnails(listView); + })); + } + finally + { + if (semaphoreAcquired) { - focusedItem = listView.Items[0]; + loadSemaphore.Release(); } - - focusedItem.Selected = true; - listView.EnsureVisible(focusedItem.Index); - - ThemeThumbLoader.CacheThumbnails(listView); - })); - loadSemaphore.Release(); + } } internal static void LoadImportedThemes(List themes, ListView listView, ImportDialog importDialog) @@ -133,13 +149,24 @@ internal static void LoadImportedThemes(List themes, ListView listV Task.Run(() => { - LoadThemes(themes, listView, new ThemeLoadOpts()); - - importDialog.Invoke(new Action(() => + try { - importDialog.thumbnailsLoaded = true; - importDialog.Close(); - })); + LoadThemes(themes, listView, new ThemeLoadOpts()); + } + catch (Exception exc) + { + LoggingHandler.LogMessage("Failed to load imported themes: {0}", exc); + } + finally + { + // Closing the dialog here rather than after LoadThemes ensures the import always finishes, even + // if generating thumbnails failed + importDialog.Invoke(new Action(() => + { + importDialog.thumbnailsLoaded = true; + importDialog.Close(); + })); + } }); }