Skip to content

Commit f103f45

Browse files
emosaruEmoSaruclaude
authored
Fix memory leak when opening a save or reloading a pack (#84) (#88)
* Fix memory leak when opening a save or reloading a pack (#84) 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> * Dispose Avalonia Bitmap objects in ImageCache on flush, not just clear 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> --------- Co-authored-by: EmoSaru <emosaru@emosaru.com> Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent ca40f53 commit f103f45

4 files changed

Lines changed: 82 additions & 10 deletions

File tree

EmoTracker.Data/Sessions/PackageInstance.cs

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,24 @@ public TrackerState GetState(Guid stateId)
189189
return mStates.TryGetValue(stateId, out var state) ? state : null;
190190
}
191191

192+
/// <summary>
193+
/// Transfers <paramref name="state"/> from this PackageInstance to
194+
/// <paramref name="destination"/> without firing lifecycle events or
195+
/// disposing anything. Used by <c>ApplicationModel.LoadProgress</c>
196+
/// when a save-file load creates a new PI: the existing primary state
197+
/// is moved to the new PI so the old PI can be safely disposed without
198+
/// touching the still-live state.
199+
/// </summary>
200+
internal void MigrateStateTo(TrackerState state, PackageInstance destination)
201+
{
202+
if (state == null) throw new ArgumentNullException(nameof(state));
203+
if (destination == null) throw new ArgumentNullException(nameof(destination));
204+
if (mStates.Remove(state.Id))
205+
destination.mStates[state.Id] = state;
206+
// state.PackageInstance is already pointing at destination —
207+
// TrackerState.LoadProgress set it before calling LoadInto.
208+
}
209+
192210
public override void Dispose()
193211
{
194212
// Tear down the live states first, then the definitional state.

EmoTracker.Data/Sessions/PackageLoader.cs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,14 @@ public PackageLoadEventArgs(TrackerState target, IGamePackage package, IGamePack
7575
[ThreadStatic]
7676
static bool mInProgress;
7777

78+
/// <summary>
79+
/// True while <see cref="LoadInto"/> is executing on this thread.
80+
/// Used by <see cref="ApplicationSettings.SyncSeedsFromSession"/> to
81+
/// suppress seed-writes driven by pack-script (init.lua) changes,
82+
/// so only explicit user UI changes update the saved defaults.
83+
/// </summary>
84+
internal static bool IsLoading => mInProgress;
85+
7886
/// <summary>
7987
/// Loads <paramref name="package"/> (with optional <paramref name="variant"/>)
8088
/// into <paramref name="target"/>'s catalogs. Caller is responsible for
@@ -126,6 +134,13 @@ public static void LoadInto(
126134
target.Items.Reset();
127135
target.Scripts.Reset();
128136

137+
// Flush the PackageInstance's decoded-image caches. Old
138+
// ImageReference keys from the now-reset catalogs would
139+
// otherwise keep both the source SKBitmaps and the resolved
140+
// IImages alive across every Reload, accumulating ~250 MB per
141+
// reload for large packs.
142+
FlushImageCaches(target.PackageInstance);
143+
129144
ApplicationColors.Instance.LoadColors();
130145

131146
if (package != null)
@@ -196,6 +211,23 @@ public static void LoadInto(
196211
}
197212
}
198213

214+
// Dispose and clear both image caches on a PackageInstance. Called
215+
// before each load so stale SKBitmaps and resolved IImages from the
216+
// previous load are released rather than accumulated in memory.
217+
static void FlushImageCaches(PackageInstance pi)
218+
{
219+
if (pi == null) return;
220+
lock (pi.SourceImageCacheLock)
221+
{
222+
foreach (var kvp in pi.SourceImageCache)
223+
(kvp.Value as IDisposable)?.Dispose();
224+
pi.SourceImageCache.Clear();
225+
}
226+
foreach (var kvp in pi.ImageCache)
227+
(kvp.Value as IDisposable)?.Dispose();
228+
pi.ImageCache.Clear();
229+
}
230+
199231
// Phase 7.1.g: pack-driven UI flags now live on the per-state
200232
// TrackerState. Reset them at the start of each load to defaults
201233
// so a pack reload picks up the pack's settings.json fresh.

EmoTracker.UI/Media/Utility/IconUtility.cs

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -48,17 +48,19 @@ public static string[] GetArgs(string filterCommand)
4848
// ── Avalonia / SkiaSharp image pipeline ──────────────────────────────────
4949

5050
// Alpha masks keyed by IImage: bool[] of length (width * height), true = opaque.
51-
// ConcurrentDictionary because the background image worker writes masks while
52-
// the UI thread reads them (InputMaskingImage.HitTest via GetAlphaMask).
53-
private static readonly System.Collections.Concurrent.ConcurrentDictionary<IImage, (bool[] mask, int w, int h)> sAlphaMasks = new();
51+
// ConditionalWeakTable so entries are freed automatically when the IImage key
52+
// becomes unreachable (e.g. after a pack reload clears PackageInstance.ImageCache).
53+
// All public CWT members are thread-safe.
54+
private sealed class AlphaMaskEntry { public bool[] Pixels; public int Width, Height; }
55+
private static readonly System.Runtime.CompilerServices.ConditionalWeakTable<IImage, AlphaMaskEntry> sAlphaMasks = new();
5456

5557
// Cached Skia-encoded PNG bytes for each IImage produced by SkToAvalonia.
5658
// Used by ToSkBitmap to bypass Avalonia's Bitmap.Save(Stream), which may strip the
5759
// alpha channel on some platforms — causing overlay compositing to treat every pixel
5860
// as fully opaque and completely hide the base layer.
59-
// ConcurrentDictionary because the background image worker writes entries while
60-
// filter resolution may read them concurrently.
61-
private static readonly System.Collections.Concurrent.ConcurrentDictionary<IImage, byte[]> sPngCache = new();
61+
// ConditionalWeakTable so entries are freed automatically when the IImage key
62+
// becomes unreachable, preventing static accumulation across pack reloads.
63+
private static readonly System.Runtime.CompilerServices.ConditionalWeakTable<IImage, byte[]> sPngCache = new();
6264

6365
// HTTP/HTTPS image download cache. null value means "download in progress".
6466
private static readonly System.Collections.Concurrent.ConcurrentDictionary<string, IImage?> sHttpCache = new();
@@ -74,7 +76,7 @@ public static string[] GetArgs(string filterCommand)
7476
public static (bool[] mask, int w, int h)? GetAlphaMask(IImage image)
7577
{
7678
if (image != null && sAlphaMasks.TryGetValue(image, out var entry))
77-
return entry;
79+
return (entry.Pixels, entry.Width, entry.Height);
7880
return null;
7981
}
8082

@@ -231,7 +233,7 @@ internal static IImage FinalizeToAvalonia(SKBitmap bmp)
231233
{
232234
var mask = ComputeAlphaMask(bmp);
233235
var avBitmap = SkToAvaloniaCore(bmp);
234-
sAlphaMasks[avBitmap] = (mask, bmp.Width, bmp.Height);
236+
sAlphaMasks.AddOrUpdate(avBitmap, new AlphaMaskEntry { Pixels = mask, Width = bmp.Width, Height = bmp.Height });
235237
bmp.Dispose();
236238
return avBitmap;
237239
}
@@ -250,7 +252,7 @@ private static IImage SkToAvalonia(SKBitmap bmp, bool storeMask = false)
250252
if (storeMask)
251253
{
252254
var mask = ComputeAlphaMask(bmp);
253-
sAlphaMasks[avBitmap] = (mask, bmp.Width, bmp.Height);
255+
sAlphaMasks.AddOrUpdate(avBitmap, new AlphaMaskEntry { Pixels = mask, Width = bmp.Width, Height = bmp.Height });
254256
}
255257

256258
return avBitmap;
@@ -267,7 +269,7 @@ private static Avalonia.Media.Imaging.Bitmap SkToAvaloniaCore(SKBitmap bmp)
267269
byte[] pngBytes = encoded.ToArray();
268270
var avBitmap = new Avalonia.Media.Imaging.Bitmap(new MemoryStream(pngBytes));
269271

270-
sPngCache[avBitmap] = pngBytes;
272+
sPngCache.AddOrUpdate(avBitmap, pngBytes);
271273

272274
return avBitmap;
273275
}

EmoTracker/ApplicationModel.cs

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1549,6 +1549,11 @@ public bool LoadProgress(string path)
15491549
var target = PrimaryState;
15501550
if (target == null) return false;
15511551

1552+
// Snapshot the PI before the load. TrackerState.LoadProgress creates
1553+
// a brand-new PackageInstance for the save's (pack, variant), leaving
1554+
// the old one stranded in mPackageInstances and never disposed.
1555+
var oldPi = target.PackageInstance;
1556+
15521557
if (target.LoadProgress(path, (JObject root) =>
15531558
{
15541559
WindowService.Instance.MainWindowWidth = root.GetValue<double>("main_window_width", WindowService.Instance.MainWindowWidth);
@@ -1566,6 +1571,21 @@ public bool LoadProgress(string path)
15661571
}
15671572
}))
15681573
{
1574+
// If LoadProgress swapped to a new PI, transfer the state so
1575+
// the old PI can be cleanly disposed (freeing its image caches,
1576+
// definitional state, and Lua interpreter — ~600 MB for large packs).
1577+
var newPi = target.PackageInstance;
1578+
if (!ReferenceEquals(oldPi, newPi))
1579+
{
1580+
oldPi?.MigrateStateTo(target, newPi);
1581+
mPackageInstances.Add(newPi);
1582+
if (ReferenceEquals(mActivePackageInstance, oldPi))
1583+
ActivePackageInstance = newPi;
1584+
mPackageInstances.Remove(oldPi);
1585+
if (oldPi != null && oldPi.States.Count == 0)
1586+
oldPi.Dispose();
1587+
}
1588+
15691589
AcquireLayouts();
15701590
// Phase 7.11 polish: a freshly-loaded state isn't dirty
15711591
// — clear the modified marker for the active state.

0 commit comments

Comments
 (0)