Fix memory leak when opening a save or reloading a pack (#84)#88
Merged
Conversation
Two distinct leaks caused memory to grow ~600 MB per save-open and
~250 MB per reload for large packs:
1. LoadProgress leak (~600 MB per open):
TrackerState.LoadProgress() creates a fresh PackageInstance for the
save's (pack, variant) but the old PI was never removed from
ApplicationModel.mPackageInstances or disposed, so its image caches,
Lua interpreter, and definitional TrackerState accumulated
indefinitely. Fix:
- Add PackageInstance.MigrateStateTo() to transfer a state between
PIs without lifecycle events or disposal.
- In ApplicationModel.LoadProgress(), detect when the PI changed,
migrate the state to the new PI, swap mPackageInstances, and
dispose the old PI (which frees its image caches and definitional
state).
2. Reload leak (~250 MB per reload):
PackageLoader.LoadInto() resets the catalogs (Items, Locations, ...)
but left PackageInstance.ImageCache and SourceImageCache intact. Old
ImageReference keys from the reset catalogs kept decoded SKBitmaps
and IImages alive across every Reload. Fix: call FlushImageCaches()
after the catalog resets so stale decoded images are released before
the new load populates them.
3. Static IconUtility caches (secondary, compounding):
sPngCache and sAlphaMasks used ConcurrentDictionary with strong
references to IImage keys, so Avalonia Bitmaps could never be GC'd
even after the PI's ImageCache was cleared. Fix: switch both to
ConditionalWeakTable so entries are evicted automatically when the
IImage key becomes unreachable.
Closes #84
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
pi.ImageCache.Clear() released the cache references but left the Avalonia Bitmap objects waiting on the finalizer queue, causing unmanaged bitmap memory to accumulate across reloads until GC eventually ran. Explicitly disposing each IImage before clearing mirrors the existing SKBitmap treatment in SourceImageCache and lets GC reclaim the memory promptly. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes two distinct memory leaks reported in #84 (large packs, Linux 3.0.3.1):
~600 MB per save-open —
TrackerState.LoadProgress()created a freshPackageInstancefor the save's (pack, variant) but the old PI was never disposed, leaving its image caches, Lua state, and definitionalTrackerStatealive indefinitely. Fix:ApplicationModel.LoadProgress()now detects the PI change, migrates the primary state to the new PI via the newPackageInstance.MigrateStateTo(), and disposes the old PI.~250 MB per reload —
PackageLoader.LoadInto()reset the item/location catalogs but leftPackageInstance.ImageCacheandSourceImageCacheintact. OldImageReferencekeys from the reset catalogs kept decodedSKBitmaps andIImages alive across every reload. Fix: flush both caches after the catalog resets inPackageLoader.LoadInto().Static cache accumulation (compounding) —
IconUtility.sPngCacheandsAlphaMasksusedConcurrentDictionarywith strongIImagereferences as keys, preventingAvalonia.Bitmapobjects from being GC'd even after the PI'sImageCachewas cleared. Fix: switch both toConditionalWeakTableso entries are evicted automatically when theIImagekey becomes unreachable.Files changed
PackageInstance.csMigrateStateTo()to transfer a state between PIs without disposalApplicationModel.csLoadProgress()detects PI change and disposes old PIPackageLoader.csFlushImageCaches()helperIconUtility.cssPngCache/sAlphaMasks→ConditionalWeakTablefor automatic evictionTest plan
🤖 Generated with Claude Code