Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
117 changes: 83 additions & 34 deletions src/UniGetUI.Avalonia/Models/PackageCollections.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,29 +40,72 @@
// reference the bitmap, so dropping it here only makes it GC-eligible.
private const int MaxIconCacheEntries = 512;
private static readonly object _iconCacheLock = new();
private static readonly Dictionary<long, LinkedListNode<(long Hash, Bitmap? Bitmap)>> _iconCache = new();
private static readonly LinkedList<(long Hash, Bitmap? Bitmap)> _iconCacheOrder = new();
private static readonly Dictionary<long, LinkedListNode<(long Hash, Bitmap Bitmap)>> _iconCache = new();
private static readonly LinkedList<(long Hash, Bitmap Bitmap)> _iconCacheOrder = new();
private static readonly Dictionary<long, long> _failedIcons = new();
Comment thread
GabrielDuf marked this conversation as resolved.
private const int MaxFailedIconEntries = 512;
private static readonly TimeSpan IconRetryInterval = TimeSpan.FromMinutes(5);

private static bool TryGetCachedIcon(long hash, out Bitmap? bitmap)
private static Bitmap? GetCachedIcon(long hash)
{
lock (_iconCacheLock)
{
if (_iconCache.TryGetValue(hash, out var node))
{
_iconCacheOrder.Remove(node);
_iconCacheOrder.AddFirst(node);
bitmap = node.Value.Bitmap;
return true;
return node.Value.Bitmap;
}
}
bitmap = null;
return false;
return null;
}

private static bool HasRecentIconFailure(long hash)
{
lock (_iconCacheLock)
{
if (!_failedIcons.TryGetValue(hash, out long failedAt))
return false;

if (Environment.TickCount64 - failedAt < (long)IconRetryInterval.TotalMilliseconds)
return true;

_failedIcons.Remove(hash);
return false;
}
}

private static void MarkIconFailed(long hash)
{
lock (_iconCacheLock)
{
_failedIcons[hash] = Environment.TickCount64;
if (_failedIcons.Count > MaxFailedIconEntries)
TrimFailedIcons();
}
}

private static void CacheIcon(long hash, Bitmap? bitmap)
private static void TrimFailedIcons()
{
long now = Environment.TickCount64;
long retryMs = (long)IconRetryInterval.TotalMilliseconds;
foreach (var expired in _failedIcons.Where(e => now - e.Value >= retryMs).ToArray())
_failedIcons.Remove(expired.Key);

int excess = _failedIcons.Count - MaxFailedIconEntries;
if (excess <= 0)
return;

foreach (var oldest in _failedIcons.OrderBy(e => e.Value).Take(excess).ToArray())
_failedIcons.Remove(oldest.Key);
}

private static void CacheIcon(long hash, Bitmap bitmap)
{
lock (_iconCacheLock)
{
_failedIcons.Remove(hash);

if (_iconCache.TryGetValue(hash, out var existing))
{
existing.Value = (hash, bitmap);
Expand All @@ -71,7 +114,7 @@
return;
}

var node = new LinkedListNode<(long, Bitmap?)>((hash, bitmap));
var node = new LinkedListNode<(long, Bitmap)>((hash, bitmap));
_iconCache[hash] = node;
_iconCacheOrder.AddFirst(node);

Expand All @@ -89,6 +132,7 @@
{
_iconCache.Clear();
_iconCacheOrder.Clear();
_failedIcons.Clear();
}
}

Expand Down Expand Up @@ -194,7 +238,7 @@
/// See issue #4617 — defense-in-depth signal that an upgrade may be redirecting the
/// download to a different domain than the user originally trusted.
/// </summary>
private void MaybeStartInstallerHostCheck()

Check warning on line 241 in src/UniGetUI.Avalonia/Models/PackageCollections.cs

View workflow job for this annotation

GitHub Actions / Linux (Avalonia)

Member 'MaybeStartInstallerHostCheck' does not access instance data and can be marked as static (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1822)

Check warning on line 241 in src/UniGetUI.Avalonia/Models/PackageCollections.cs

View workflow job for this annotation

GitHub Actions / Linux (NativeAOT)

Member 'MaybeStartInstallerHostCheck' does not access instance data and can be marked as static (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1822)
{
#if WINDOWS
if (!Package.IsUpgradable) return;
Expand Down Expand Up @@ -265,18 +309,17 @@
{
CancellationToken token = _lifetimeCts.Token;
long hash = Package.GetHash();
if (TryGetCachedIcon(hash, out Bitmap? cached))
if (GetCachedIcon(hash) is { } cached)
{
if (cached is not null)
await Dispatcher.UIThread.InvokeAsync(() =>
{
await Dispatcher.UIThread.InvokeAsync(() =>
{
if (!token.IsCancellationRequested) IconBitmap = cached;
});
}
if (!token.IsCancellationRequested) IconBitmap = cached;
});
return;
}

if (HasRecentIconFailure(hash)) return;

try
{
Bitmap? bitmap = await GetSharedIconLoad(hash, Package).WaitAsync(token).ConfigureAwait(false);
Expand All @@ -289,15 +332,17 @@
});
}
catch (OperationCanceledException) { /* row discarded before its icon finished loading */ }
catch { CacheIcon(hash, null); }
catch { MarkIconFailed(hash); }
}

private static Task<Bitmap?> GetSharedIconLoad(long hash, IPackage package)
{
lock (_inflightIconLoadsLock)
{
if (TryGetCachedIcon(hash, out Bitmap? cached))
return Task.FromResult(cached);
if (GetCachedIcon(hash) is { } cached)
return Task.FromResult<Bitmap?>(cached);
if (HasRecentIconFailure(hash))
return Task.FromResult<Bitmap?>(null);
if (_inflightIconLoads.TryGetValue(hash, out Task<Bitmap?>? existing))
return existing;

Expand Down Expand Up @@ -328,14 +373,14 @@
try
{
var uri = await Task.Run(package.GetIconUrlIfAny).ConfigureAwait(false);
if (uri is null) { CacheIcon(hash, null); return null; }
if (uri is null) { MarkIconFailed(hash); return null; }

Bitmap? decoded;
if (uri.IsFile)
{
if (!IsSkiaDecodableExtension(uri.LocalPath))
if (IsKnownUndecodableExtension(uri.LocalPath))
{
CacheIcon(hash, null);
MarkIconFailed(hash);
return null;
}
decoded = await Task.Run(() => TryDecodeIcon(uri.LocalPath)).ConfigureAwait(false);
Expand All @@ -348,22 +393,28 @@
response.EnsureSuccessStatusCode();
if (response.Content.Headers.ContentLength > MaxIconDownloadBytes)
{
CacheIcon(hash, null);
MarkIconFailed(hash);
return null;
}

await response.Content.LoadIntoBufferAsync(MaxIconDownloadBytes).ConfigureAwait(false);
var bytes = await response.Content.ReadAsByteArrayAsync().ConfigureAwait(false);
decoded = TryDecodeIcon(bytes, uri.Host);
}
else { CacheIcon(hash, null); return null; }
else { MarkIconFailed(hash); return null; }

if (decoded is null)
{
MarkIconFailed(hash);
return null;
}

CacheIcon(hash, decoded);
return decoded;
}
catch
{
CacheIcon(hash, null);
MarkIconFailed(hash);
return null;
}
finally
Expand All @@ -373,7 +424,7 @@
}

// Icons come from a shared on-disk cache that can hold empty or partial entries after an
// interrupted download; decoding those throws. Skip them quietly instead of surfacing an error.
// interrupted download; decoding those throws. Skip those entries instead of failing the row.
private static Bitmap? TryDecodeIcon(string filePath)
{
var info = new FileInfo(filePath);
Expand All @@ -387,23 +438,21 @@
private static Bitmap? TryDecodeIcon(Func<Bitmap> decode, string source)
{
try { return decode(); }
catch (Exception ex) { Logger.Debug($"Discarding undecodable icon '{source}': {ex.Message}"); return null; }
catch (Exception ex) { Logger.Warn($"Discarding undecodable icon '{source}': {ex.Message}"); return null; }
}

// Decode directly at the display-cache width. This avoids allocating a full-size bitmap first,
// which is important for untrusted or unusually large package artwork.
private static Bitmap DecodeDownscaled(Stream stream)
=> Bitmap.DecodeToWidth(stream, MaxIconSide, BitmapInterpolationMode.HighQuality);

private static bool IsSkiaDecodableExtension(string path)
private static bool IsKnownUndecodableExtension(string path)
{
string ext = Path.GetExtension(path);
return ext.Equals(".png", StringComparison.OrdinalIgnoreCase)
|| ext.Equals(".jpg", StringComparison.OrdinalIgnoreCase)
|| ext.Equals(".jpeg", StringComparison.OrdinalIgnoreCase)
|| ext.Equals(".gif", StringComparison.OrdinalIgnoreCase)
|| ext.Equals(".bmp", StringComparison.OrdinalIgnoreCase)
|| ext.Equals(".webp", StringComparison.OrdinalIgnoreCase);
return ext.Equals(".svg", StringComparison.OrdinalIgnoreCase)
|| ext.Equals(".tif", StringComparison.OrdinalIgnoreCase)
|| ext.Equals(".tiff", StringComparison.OrdinalIgnoreCase)
|| ext.Equals(".avif", StringComparison.OrdinalIgnoreCase);
}

private void Package_PropertyChanged(object? sender, PropertyChangedEventArgs e)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ private async Task ResetIconCache(Visual? _)
try { Directory.Delete(CoreData.UniGetUICacheDirectory_Icons, true); }
catch (Exception ex) { Logger.Error(ex); }
global::UniGetUI.PackageEngine.PackageClasses.PackageWrapper.ClearIconCache();
global::UniGetUI.PackageEngine.PackageClasses.Package.ResetIconCache();
RestartRequired?.Invoke(this, EventArgs.Empty);
await LoadIconCacheSize();
}
Expand Down
4 changes: 2 additions & 2 deletions src/UniGetUI.Core.IconStore/IconCacheEngine.cs
Original file line number Diff line number Diff line change
Expand Up @@ -455,7 +455,7 @@ private static void DeteteCachedFiles(string iconLocation)
{ "image/svg+xml", "svg" },
{ "image/vnd.microsoft.icon", "ico" },
{ "application/octet-stream", "ico" },
{ "image/image/x-icon", "ico" },
{ "image/x-icon", "ico" },
{ "image/tiff", "tif" },
}
);
Expand All @@ -471,7 +471,7 @@ private static void DeteteCachedFiles(string iconLocation)
{ "png", "image/png" },
{ "webp", "image/webp" },
{ "svg", "image/svg+xml" },
{ "ico", "image/image/x-icon" },
{ "ico", "image/x-icon" },
{ "tif", "image/tiff" },
}
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,14 @@ public partial class Package : IPackage
private readonly string _ignoredId;
private readonly string _iconId;

private static readonly ConcurrentDictionary<int, Uri?> _cachedIconPaths = new();
private static readonly ConcurrentDictionary<long, Uri> _cachedIconPaths = new();
private static readonly ConcurrentDictionary<long, long> _failedIconLookups = new();
private static readonly TimeSpan _iconLookupRetryInterval = TimeSpan.FromMinutes(5);

public static TimeSpan? TEST_IconLookupRetryIntervalOverride { private get; set; }

private static TimeSpan IconLookupRetryInterval =>
TEST_IconLookupRetryIntervalOverride ?? _iconLookupRetryInterval;

private IPackageDetails? __details;
public IPackageDetails Details
Expand Down Expand Up @@ -162,12 +169,30 @@ public virtual Uri GetIconUrl()

public virtual Uri? GetIconUrlIfAny()
{
if (_cachedIconPaths.TryGetValue(this.GetHashCode(), out Uri? path))
long cacheKey = _versionedHash;
if (_cachedIconPaths.TryGetValue(cacheKey, out Uri? path))
{
return path;
}

if (
_failedIconLookups.TryGetValue(cacheKey, out long failedAt)
&& Environment.TickCount64 - failedAt
< (long)IconLookupRetryInterval.TotalMilliseconds
)
{
return null;
}

var CachedIcon = LoadIconUrlIfAny();
_cachedIconPaths.TryAdd(this.GetHashCode(), CachedIcon);
if (CachedIcon is null)
{
_failedIconLookups[cacheKey] = Environment.TickCount64;
return null;
}

_failedIconLookups.TryRemove(cacheKey, out _);
_cachedIconPaths[cacheKey] = CachedIcon;
return CachedIcon;
}

Expand Down Expand Up @@ -377,6 +402,7 @@ public SerializableIncompatiblePackage AsSerializable_Incompatible()
public static void ResetIconCache()
{
_cachedIconPaths.Clear();
_failedIconLookups.Clear();
}

private static string GenerateIconId(Package p)
Expand Down
Loading
Loading