From 9a134d43cb631cbda99ff45d5a7d3d1a3e157ae5 Mon Sep 17 00:00:00 2001 From: rmbakker88 Date: Sun, 26 Jul 2026 01:15:34 +0200 Subject: [PATCH 1/2] 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(); + })); + } }); } From 324658af83af4566ca7c52200f04da694075a68f Mon Sep 17 00:00:00 2001 From: Timothy Johnson Date: Fri, 9 Oct 2026 00:03:38 -0400 Subject: [PATCH 2/2] Address Copilot feedback --- src/ThemeDialogUtils.cs | 74 +++++++++++++++++++++-------------------- 1 file changed, 38 insertions(+), 36 deletions(-) diff --git a/src/ThemeDialogUtils.cs b/src/ThemeDialogUtils.cs index 81050e41..ae540223 100644 --- a/src/ThemeDialogUtils.cs +++ b/src/ThemeDialogUtils.cs @@ -1,4 +1,4 @@ -// This Source Code Form is subject to the terms of the Mozilla Public +// This Source Code Form is subject to the terms of the Mozilla Public // License, v. 2.0. If a copy of the MPL was not distributed with this // file, You can obtain one at http://mozilla.org/MPL/2.0/. @@ -54,7 +54,11 @@ internal static string[] GetDisplayNames() internal static void LoadThemes(List themes, ListView listView, ThemeLoadOpts opts) { - bool semaphoreAcquired = loadSemaphore.Wait(60000); + if (!loadSemaphore.Wait(60000)) + { + LoggingHandler.LogMessage("Timed out waiting for theme load semaphore"); + return; + } try { @@ -68,43 +72,45 @@ internal static void LoadThemes(List themes, ListView listView, The continue; } + Image thumbnailImage; try { - using (Image thumbnailImage = ThemeThumbLoader.GetThumbnailImage(theme, thumbnailSize, true)) - { - 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; - - 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; - } - })); - } + thumbnailImage = ThemeThumbLoader.GetThumbnailImage(theme, thumbnailSize, true); } 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 + // Unsupported or corrupt images can throw more than OutOfMemoryException. LoggingHandler.LogMessage("Failed to generate thumbnail for '{0}' theme: {1}", theme.themeId, exc); listView.Invoke(new Action(() => ThemeLoader.HandleError(new FailedToCreateThumbnail(theme.themeId)))); + continue; + } + + using (thumbnailImage) + { + 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; + + 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; + } + })); } } @@ -125,10 +131,7 @@ internal static void LoadThemes(List themes, ListView listView, The } finally { - if (semaphoreAcquired) - { - loadSemaphore.Release(); - } + loadSemaphore.Release(); } } @@ -161,8 +164,7 @@ internal static void LoadImportedThemes(List themes, ListView listV } finally { - // Closing the dialog here rather than after LoadThemes ensures the import always finishes, even - // if generating thumbnails failed + // Close the import dialog even if thumbnail loading fails. importDialog.Invoke(new Action(() => { importDialog.thumbnailsLoaded = true;