From 3ec8ab15903ed8c21a1151bf9994ac1f69cf9086 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 6 Oct 2026 00:25:03 +0000 Subject: [PATCH] fix: release RingBuffer elements as PushBack evicts them [patch] PushBack advanced FrontIndex past the evicted element without clearing its slot. When Length is not a power of two the backing array is larger than the live window, so up to Length-1 evicted elements stayed reachable until their slot happened to be overwritten again. Clear the evicted slot before advancing, then write the new element, so the Length == Capacity case (where the evicted slot is the one being written) still keeps the new element. Fixes ktsu-dev/Containers#85 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01UoSUYMQMykgfsb5y6AhnKb --- Containers.Test/RingBufferTests.cs | 33 ++++++++++++++++++++++++++++++ Containers/RingBuffer.cs | 9 +++++++- 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/Containers.Test/RingBufferTests.cs b/Containers.Test/RingBufferTests.cs index 061d089..4a31394 100644 --- a/Containers.Test/RingBufferTests.cs +++ b/Containers.Test/RingBufferTests.cs @@ -275,6 +275,39 @@ public void Clear_ReleasesReferencesToClearedElements() Assert.IsFalse(references.Any(r => r.IsAlive)); } + [TestMethod] + public void PushBack_ReleasesReferenceToEvictedElement() + { + // Length 5 is backed by a capacity of 8, so the evicted slot is not overwritten by the next push + RingBuffer buffer = new(5); + WeakReference[] references = FillWithUnreferencedObjects(buffer, 1); + for (int i = 0; i < 5; i++) + { + buffer.PushBack(i); + } + + GC.Collect(); + GC.WaitForPendingFinalizers(); + GC.Collect(); + + object[] expected = [0, 1, 2, 3, 4]; + Assert.AreSequenceEqual(expected, buffer); + Assert.IsFalse(references[0].IsAlive); + } + + [TestMethod] + public void PushBack_FullPowerOfTwoBuffer_KeepsNewestElements() + { + // Length equals capacity, so the evicted front slot is the one the new element is written to + RingBuffer buffer = new([1, 2, 3, 4], 4); + buffer.PushBack(5); + buffer.PushBack(6); + + Assert.AreSequenceEqual(Enumerable.Range(3, 4), buffer); + Assert.AreEqual(3, buffer.Front()); + Assert.AreEqual(6, buffer.Back()); + } + [MethodImpl(MethodImplOptions.NoInlining)] private static WeakReference[] FillWithUnreferencedObjects(RingBuffer buffer, int count) { diff --git a/Containers/RingBuffer.cs b/Containers/RingBuffer.cs index 635a0ea..15a659e 100644 --- a/Containers/RingBuffer.cs +++ b/Containers/RingBuffer.cs @@ -158,9 +158,13 @@ private void AllocateBuffer(int length) /// The element to add. public void PushBack(T o) { - Buffer[BackIndex] = o; if (Count == Length) { + // Clear the evicted slot so its element can be collected. When Length is not a power of two + // the slot lies outside the live window and would otherwise keep the element alive until it + // is next overwritten. + Buffer[FrontIndex] = default!; + // Advance front index with wraparound using bitwise AND for efficiency FrontIndex = (FrontIndex + 1) & (Capacity - 1); } @@ -169,6 +173,9 @@ public void PushBack(T o) Count++; } + // Write after clearing, because when Length equals Capacity the evicted slot is this one + Buffer[BackIndex] = o; + // Advance back index with wraparound using bitwise AND for efficiency BackIndex = (BackIndex + 1) & (Capacity - 1); }