Skip to content

Prevent cache writer use-after-free during cleanup - #2550

Merged
ge0rdi merged 1 commit into
Open-Shell:masterfrom
YellowNest:fix/cache-save-snapshot-lifetime
Oct 4, 2026
Merged

ge0rdi merged 1 commit into
Open-Shell:masterfrom
YellowNest:fix/cache-save-snapshot-lifetime

Conversation

@YellowNest

Copy link
Copy Markdown

Summary

Prevent the asynchronous cache writer from retaining pointers to entries that normal cache cleanup can erase, and serialize explicit cache clearing with an in-flight save.

Why

SaveCacheFileThread first snapshots raw pointers to m_IconInfos and m_ItemInfos under read locks, releases those locks, then reacquires a read lock for each pointer while serializing it.

The later lock cannot make a pointer valid again if the entry was erased between the snapshot and that reacquisition.

ResetTempIcons() can erase temporary items and temporary/Metro icons during that window. ClearCache() can erase every cached item and icon while a save is in progress.

The save path already skips temporary and Metro entries when it eventually serializes them. This change applies those same exclusions while the snapshot lock is still held, so pointers to entries that ResetTempIcons() may erase are never retained. ClearCache() waits for an in-flight cache writer before deleting the persistent containers and cache file.

This also ensures an already-running save cannot recreate DataCache.db after an explicit cache clear.

Verification

  • GitHub Actions Build completed successfully for exact commit c2c47b4b3b5d4ae8fd0dcb98e424a0e5877d6338 in run 37158312012.
  • A standalone C++20 ASan/UBSan regression harness deterministically reproduces heap-use-after-free with the original snapshot/erase sequence for both temporary cleanup and full cache clearing; patched sequencing completes without sanitizer errors.
  • 2,000,000 randomized entry combinations produced 0 differences in which icon/item entries are effectively serialized before vs. after the snapshot filtering change.
  • Audited the m_ItemInfos / m_IconInfos erase/clear paths: runtime cleanup removes only entries now excluded from snapshots; full cache clearing is explicitly synchronized; shutdown already waits for the save thread.

This is a source-identified lifetime bug; it is not being attributed to a specific reported crash.

@ge0rdi
ge0rdi merged commit 2e2f681 into Open-Shell:master Oct 4, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants