Skip to content

Conversation

@workbysaran
Copy link
Contributor

Resolved / Related Issues

Addresses performance issues caused by repeated Shell API thumbnail generation by introducing a disk based LRU cache and a centralized ThumbnailService.

Steps used to test these changes

  1. Navigate to a folder with mixed file types (images, PDFs, etc.) and verify that thumbnails are generated and cached
  2. Scroll down and then back up, and verify that thumbnails load instantly from the cache
  3. Navigate away from the folder and return to it, and verify that thumbnails load instantly from the cache
  4. Restart the app, navigate to the same folder, and verify that thumbnails load instantly from the cache
  5. Modify an image file and verify that the thumbnail is regenerated
  6. Switch between different views and verify that thumbnails are generated correctly for each view

_memoryIndex = new ConcurrentDictionary<string, CacheEntry>();
_evictionLock = new SemaphoreSlim(1, 1);

_ = LoadCacheIndexAsync();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: A race condition on startup causes the thumbnail cache to be ignored. LoadCacheIndexAsync runs in the background but UI may request thumbnails before the cache index is loaded.
Severity: HIGH | Confidence: High

🔍 Detailed Analysis

A race condition exists during application startup. The ThumbnailCache constructor initiates a fire-and-forget background task, LoadCacheIndexAsync, to populate its in-memory index from the disk cache. Simultaneously, the main application initialization InitializeApplicationAsync also starts. If the main application requests a thumbnail before LoadCacheIndexAsync completes, the in-memory index _memoryIndex will be empty. This results in a cache miss, forcing unnecessary regeneration of thumbnails and defeating the purpose of the persistent cache on the first navigation after startup.

💡 Suggested Fix

Ensure that the cache index is fully loaded before any thumbnail requests can be processed. This can be achieved by making LoadCacheIndexAsync an awaitable task and ensuring the application's initialization logic awaits its completion before proceeding with UI loading that might trigger thumbnail generation.

🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: src/Files.App/Services/Thumbnails/ThumbnailCache.cs#L39

Potential issue: A race condition exists during application startup. The
`ThumbnailCache` constructor initiates a fire-and-forget background task,
`LoadCacheIndexAsync`, to populate its in-memory index from the disk cache.
Simultaneously, the main application initialization `InitializeApplicationAsync` also
starts. If the main application requests a thumbnail before `LoadCacheIndexAsync`
completes, the in-memory index `_memoryIndex` will be empty. This results in a cache
miss, forcing unnecessary regeneration of thumbnails and defeating the purpose of the
persistent cache on the first navigation after startup.

Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 7910670

@yaira2 yaira2 self-requested a review December 24, 2025 15:43
@yaira2 yaira2 added the ready for review Pull requests that are ready for review label Dec 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready for review Pull requests that are ready for review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants