From 177b4925a123ea8a675307296e38f665b877e946 Mon Sep 17 00:00:00 2001 From: gamer147 Date: Thu, 28 May 2026 21:13:47 -0400 Subject: [PATCH] =?UTF-8?q?fix(session):=20bound=20the=20SID=E2=86=92UDID?= =?UTF-8?q?=20dict=20with=20FIFO=20eviction?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The map used to grow unbounded over the process's lifetime — every fresh signup added an entry that was never reclaimed. Long-running dev hosts (or any future emulator deployment that doesn't restart often) would gradually leak memory. Cap at 10k entries by default with a simple FIFO eviction queue; re-stores of the same SID don't grow the queue. Co-Authored-By: Claude Opus 4.7 --- .../Services/ShadowverseSessionService.cs | 42 ++++++++++++++-- .../ShadowverseSessionServiceTests.cs | 50 +++++++++++++++++++ 2 files changed, 89 insertions(+), 3 deletions(-) diff --git a/SVSim.EmulatedEntrypoint/Services/ShadowverseSessionService.cs b/SVSim.EmulatedEntrypoint/Services/ShadowverseSessionService.cs index 0ed8612..1bfa09d 100644 --- a/SVSim.EmulatedEntrypoint/Services/ShadowverseSessionService.cs +++ b/SVSim.EmulatedEntrypoint/Services/ShadowverseSessionService.cs @@ -14,11 +14,27 @@ public class ShadowverseSessionService /// private const string MakeMd5Salt = "r!I@ws8e5i="; - private readonly ConcurrentDictionary _sessionIdToUdid; + /// + /// Default cap for the in-memory SID→UDID map. Each entry is roughly 32B SID + 16B Guid + /// plus dict + queue overhead — 10k entries ≈ 1 MB of process memory. Sized for the + /// emulator's expected ceiling, not prod scale. Long-running dev hosts that keep + /// accumulating signups would otherwise grow this dict unboundedly. + /// + public const int DefaultMaxEntries = 10_000; - public ShadowverseSessionService() + private readonly int _maxEntries; + private readonly ConcurrentDictionary _sessionIdToUdid; + private readonly ConcurrentQueue _insertionOrder; + + public ShadowverseSessionService() : this(DefaultMaxEntries) { } + + public ShadowverseSessionService(int maxEntries) { + if (maxEntries <= 0) + throw new ArgumentOutOfRangeException(nameof(maxEntries), "Cap must be positive."); + _maxEntries = maxEntries; _sessionIdToUdid = new(); + _insertionOrder = new(); } public Guid? GetUdidFromSessionId(string sid) @@ -33,7 +49,27 @@ public class ShadowverseSessionService public void StoreUdidForSessionId(string sid, Guid udid) { - _sessionIdToUdid.AddOrUpdate(sid, _ => udid, (_, _) => udid); + // FIFO eviction: only enqueue on first insertion so the queue doesn't grow when + // an existing SID is re-stored (the only realistic "update" — same SID always + // resolves to the same UDID by construction of ComputeClientSessionId, so this + // path is effectively a no-op semantically). + if (_sessionIdToUdid.TryAdd(sid, udid)) + { + _insertionOrder.Enqueue(sid); + EvictIfOverCap(); + } + else + { + _sessionIdToUdid[sid] = udid; + } + } + + private void EvictIfOverCap() + { + while (_sessionIdToUdid.Count > _maxEntries && _insertionOrder.TryDequeue(out var oldest)) + { + _sessionIdToUdid.TryRemove(oldest, out _); + } } /// diff --git a/SVSim.UnitTests/Services/ShadowverseSessionServiceTests.cs b/SVSim.UnitTests/Services/ShadowverseSessionServiceTests.cs index 4f6e511..b2672f7 100644 --- a/SVSim.UnitTests/Services/ShadowverseSessionServiceTests.cs +++ b/SVSim.UnitTests/Services/ShadowverseSessionServiceTests.cs @@ -35,4 +35,54 @@ public class ShadowverseSessionServiceTests Assert.That(svc.GetUdidFromSessionId("dc4aac79d35fe15dfb6262e0071bb03c"), Is.EqualTo(udid)); } + + [Test] + public void StoreUdidForSessionId_evicts_oldest_when_cap_exceeded() + { + // Cap=3, insert 5 distinct SIDs; the two earliest must be evicted. + var svc = new ShadowverseSessionService(maxEntries: 3); + var udid = new System.Guid("62747917-93bc-454c-abb4-ef423b3c9317"); + + svc.StoreUdidForSessionId("sid-1", udid); + svc.StoreUdidForSessionId("sid-2", udid); + svc.StoreUdidForSessionId("sid-3", udid); + svc.StoreUdidForSessionId("sid-4", udid); + svc.StoreUdidForSessionId("sid-5", udid); + + Assert.That(svc.GetUdidFromSessionId("sid-1"), Is.Null, "Oldest entry must be evicted."); + Assert.That(svc.GetUdidFromSessionId("sid-2"), Is.Null, "Second-oldest entry must be evicted."); + Assert.That(svc.GetUdidFromSessionId("sid-3"), Is.EqualTo(udid)); + Assert.That(svc.GetUdidFromSessionId("sid-4"), Is.EqualTo(udid)); + Assert.That(svc.GetUdidFromSessionId("sid-5"), Is.EqualTo(udid)); + } + + [Test] + public void StoreUdidForSessionId_re_storing_same_sid_does_not_grow_queue() + { + // Cap=2. Store sid-A, then re-store sid-A many times, then store sid-B and sid-C. + // The re-stores must NOT count toward the cap — sid-A should still resolve after + // sid-B and sid-C land, because only two distinct SIDs are tracked. + var svc = new ShadowverseSessionService(maxEntries: 2); + var udid = new System.Guid("62747917-93bc-454c-abb4-ef423b3c9317"); + + svc.StoreUdidForSessionId("sid-A", udid); + for (int i = 0; i < 20; i++) svc.StoreUdidForSessionId("sid-A", udid); + svc.StoreUdidForSessionId("sid-B", udid); + + Assert.That(svc.GetUdidFromSessionId("sid-A"), Is.EqualTo(udid), "sid-A must still resolve after re-stores."); + Assert.That(svc.GetUdidFromSessionId("sid-B"), Is.EqualTo(udid)); + + // sid-C pushes us over the cap → sid-A (oldest) evicted. + svc.StoreUdidForSessionId("sid-C", udid); + Assert.That(svc.GetUdidFromSessionId("sid-A"), Is.Null); + Assert.That(svc.GetUdidFromSessionId("sid-B"), Is.EqualTo(udid)); + Assert.That(svc.GetUdidFromSessionId("sid-C"), Is.EqualTo(udid)); + } + + [Test] + public void Constructor_rejects_non_positive_cap() + { + Assert.Throws(() => new ShadowverseSessionService(maxEntries: 0)); + Assert.Throws(() => new ShadowverseSessionService(maxEntries: -1)); + } }