diff --git a/Containers.Test/SlotClearingTests.cs b/Containers.Test/SlotClearingTests.cs new file mode 100644 index 0000000..9e29535 --- /dev/null +++ b/Containers.Test/SlotClearingTests.cs @@ -0,0 +1,166 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.Containers.Tests; + +using System.Runtime.CompilerServices; +using System.Text.RegularExpressions; +using Microsoft.VisualStudio.TestTools.UnitTesting; + +/// +/// Guards that removed and cleared slots stop keeping their contents alive, including for structs that +/// hold references, which a plain value-type check misses on the netstandard builds (issue #79). +/// +[TestClass] +public partial class SlotClearingTests +{ + [GeneratedRegex(@"if\s*\(\s*!\s*typeof\([^)]*\)\.IsValueType\s*\)")] + private static partial Regex ValueTypeGuard(); + + [TestMethod] + public void IsNeeded_IsTrueForReferencesAndStructsHoldingThem() + { + Assert.IsTrue(SlotClearing.IsNeeded()); + Assert.IsTrue(SlotClearing.IsNeeded>()); + Assert.IsTrue(SlotClearing.IsNeeded<(string, int)>()); + } + + [TestMethod] + public void IsNeeded_IsFalseForUnmanagedTypesWhereTheRuntimeCanTell() => + Assert.IsFalse(SlotClearing.IsNeeded()); + + [TestMethod] + public void ContiguousMap_Clear_ReleasesValues() + { + ContiguousMap map = []; + WeakReference[] references = FillMap(map, 3); + + map.Clear(); + + AssertAllCollected(references); + } + + [TestMethod] + public void ContiguousMap_RemovingEveryKey_ReleasesValues() + { + ContiguousMap map = []; + WeakReference[] references = FillMap(map, 3); + + for (int i = 0; i < references.Length; i++) + { + Assert.IsTrue(map.Remove(i)); + } + + AssertAllCollected(references); + } + + [TestMethod] + public void ContiguousCollection_OfReferenceHoldingStructs_Clear_ReleasesValues() + { + ContiguousCollection> collection = []; + WeakReference[] references = FillCollection(collection, 3); + + collection.Clear(); + + AssertAllCollected(references); + } + + [TestMethod] + public void ContiguousCollection_OfReferenceHoldingStructs_Remove_ReleasesValue() + { + ContiguousCollection> collection = []; + WeakReference[] references = FillCollection(collection, 1); + + collection.RemoveAt(0); + + AssertAllCollected(references); + } + + [TestMethod] + public void ContiguousSet_OfReferenceHoldingStructs_Clear_ReleasesValues() + { + ContiguousSet> set = []; + WeakReference[] references = FillSet(set, 3); + + set.Clear(); + + AssertAllCollected(references); + } + + /// + /// The test project only runs on net10, where the runtime check is always available, so this guards + /// the netstandard fallback at the source: every container must go through + /// rather than test IsValueType itself. + /// + [TestMethod] + public void Containers_DoNotDecideSlotClearingWithAValueTypeCheck() + { + DirectoryInfo? directory = new(AppContext.BaseDirectory); + while (directory is not null && !File.Exists(Path.Combine(directory.FullName, "Containers.sln"))) + { + directory = directory.Parent; + } + + Assert.IsNotNull(directory, $"Could not locate Containers.sln above '{AppContext.BaseDirectory}'."); + + string[] offenders = + [ + .. Directory.EnumerateFiles(Path.Combine(directory.FullName, "Containers"), "*.cs") + .Where(file => ValueTypeGuard().IsMatch(File.ReadAllText(file))) + .Select(Path.GetFileName) + .OfType() + ]; + + Assert.IsEmpty(offenders, $"Use SlotClearing.IsNeeded() instead of an IsValueType check in: {string.Join(", ", offenders)}"); + } + + private static void AssertAllCollected(WeakReference[] references) + { + GC.Collect(); + GC.WaitForPendingFinalizers(); + GC.Collect(); + + Assert.IsFalse(references.Any(r => r.IsAlive)); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static WeakReference[] FillMap(ContiguousMap map, int count) + { + WeakReference[] references = new WeakReference[count]; + for (int i = 0; i < count; i++) + { + object value = new(); + map.Add(i, value); + references[i] = new WeakReference(value); + } + + return references; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static WeakReference[] FillCollection(ContiguousCollection> collection, int count) + { + WeakReference[] references = new WeakReference[count]; + for (int i = 0; i < count; i++) + { + object value = new(); + collection.Add(new KeyValuePair(i, value)); + references[i] = new WeakReference(value); + } + + return references; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + private static WeakReference[] FillSet(ContiguousSet> set, int count) + { + WeakReference[] references = new WeakReference[count]; + for (int i = 0; i < count; i++) + { + object value = new(); + set.Add(new KeyValuePair(i, value)); + references[i] = new WeakReference(value); + } + + return references; + } +} diff --git a/Containers/ContiguousCollection.cs b/Containers/ContiguousCollection.cs index b315cf8..be6f72f 100644 --- a/Containers/ContiguousCollection.cs +++ b/Containers/ContiguousCollection.cs @@ -4,9 +4,6 @@ namespace ktsu.Containers; using System.Collections; using System.Diagnostics.CodeAnalysis; -#if NET5_0_OR_GREATER -using System.Runtime.CompilerServices; -#endif /// /// Represents a generic collection that guarantees contiguous memory allocation for optimal cache performance. @@ -172,11 +169,7 @@ public void Add(T item) /// public void Clear() { -#if NET5_0_OR_GREATER - if (RuntimeHelpers.IsReferenceOrContainsReferences()) -#else - if (!typeof(T).IsValueType) -#endif + if (SlotClearing.IsNeeded()) { // Clear references to help GC Array.Clear(items, 0, Count); @@ -255,11 +248,7 @@ public void RemoveAt(int index) Array.Copy(items, index + 1, items, index, Count - index); } -#if NET5_0_OR_GREATER - if (RuntimeHelpers.IsReferenceOrContainsReferences()) -#else - if (!typeof(T).IsValueType) -#endif + if (SlotClearing.IsNeeded()) { items[Count] = default!; } diff --git a/Containers/ContiguousMap.cs b/Containers/ContiguousMap.cs index 7266412..3f3a0d4 100644 --- a/Containers/ContiguousMap.cs +++ b/Containers/ContiguousMap.cs @@ -4,9 +4,6 @@ namespace ktsu.Containers; using System.Collections; using System.Diagnostics.CodeAnalysis; -#if NET5_0_OR_GREATER -using System.Runtime.CompilerServices; -#endif /// /// Represents a generic map/dictionary that maintains key-value pairs in contiguous memory for optimal cache performance. @@ -389,11 +386,7 @@ public bool Remove(TKey key) Array.Copy(items, index + 1, items, index, Count - index); } -#if NET5_0_OR_GREATER - if (RuntimeHelpers.IsReferenceOrContainsReferences()) -#else - if (!typeof(Entry).IsValueType) -#endif + if (SlotClearing.IsNeeded()) { items[Count] = default; } @@ -462,11 +455,7 @@ public bool TryGetValue(TKey key, out TValue value) public void Clear() { version++; -#if NET5_0_OR_GREATER - if (RuntimeHelpers.IsReferenceOrContainsReferences()) -#else - if (!typeof(Entry).IsValueType) -#endif + if (SlotClearing.IsNeeded()) { // Clear references to help GC Array.Clear(items, 0, Count); diff --git a/Containers/ContiguousSet.cs b/Containers/ContiguousSet.cs index 947f6f5..cffdd28 100644 --- a/Containers/ContiguousSet.cs +++ b/Containers/ContiguousSet.cs @@ -4,9 +4,6 @@ namespace ktsu.Containers; using System.Collections; using System.Diagnostics.CodeAnalysis; -#if NET5_0_OR_GREATER -using System.Runtime.CompilerServices; -#endif /// /// Represents a generic set that maintains unique elements in contiguous memory for optimal cache performance. @@ -228,11 +225,7 @@ public bool Add(T item) /// public void Clear() { -#if NET5_0_OR_GREATER - if (RuntimeHelpers.IsReferenceOrContainsReferences()) -#else - if (!typeof(T).IsValueType) -#endif + if (SlotClearing.IsNeeded()) { // Clear references to help GC Array.Clear(items, 0, Count); @@ -310,11 +303,7 @@ public bool Remove(T item) Array.Copy(items, index + 1, items, index, Count - index); } -#if NET5_0_OR_GREATER - if (RuntimeHelpers.IsReferenceOrContainsReferences()) -#else - if (!typeof(T).IsValueType) -#endif + if (SlotClearing.IsNeeded()) { items[Count] = default!; } diff --git a/Containers/RingBuffer.cs b/Containers/RingBuffer.cs index 635a0ea..6a705ea 100644 --- a/Containers/RingBuffer.cs +++ b/Containers/RingBuffer.cs @@ -5,9 +5,6 @@ namespace ktsu.Containers; using System.Collections; using System.Diagnostics.CodeAnalysis; using System.Linq; -#if NET5_0_OR_GREATER -using System.Runtime.CompilerServices; -#endif /// /// Represents a fixed-size circular buffer (ring buffer) for storing elements of type . @@ -307,11 +304,7 @@ public IEnumerator GetEnumerator() /// public void Clear() { -#if NET5_0_OR_GREATER - if (RuntimeHelpers.IsReferenceOrContainsReferences()) -#else - if (!typeof(T).IsValueType) -#endif + if (SlotClearing.IsNeeded()) { // Clear references so cleared elements can be collected Array.Clear(Buffer, 0, Buffer.Length); diff --git a/Containers/SlotClearing.cs b/Containers/SlotClearing.cs new file mode 100644 index 0000000..782f331 --- /dev/null +++ b/Containers/SlotClearing.cs @@ -0,0 +1,32 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.Containers; + +#if NETSTANDARD2_1_OR_GREATER || NETCOREAPP2_0_OR_GREATER +using System.Runtime.CompilerServices; +#endif + +/// +/// Decides whether a vacated array slot has to be reset so the garbage collector can reclaim what it held. +/// +internal static class SlotClearing +{ + /// + /// Returns whether a slot of type can keep an object alive, and so must be + /// cleared when an element is removed or the container is cleared. + /// + /// + /// RuntimeHelpers.IsReferenceOrContainsReferences exists from netstandard2.1 and netcoreapp2.0. + /// On netstandard2.0 there is no cheap equivalent, so the answer is always true: clearing a slot that + /// holds no references costs little, while skipping one that does leaks. A plain value-type check is + /// not enough there, because a struct such as KeyValuePair<int, object> holds a reference. + /// + /// The slot's element type. + /// True when the slot must be cleared. + internal static bool IsNeeded() => +#if NETSTANDARD2_1_OR_GREATER || NETCOREAPP2_0_OR_GREATER + RuntimeHelpers.IsReferenceOrContainsReferences(); +#else + true; +#endif +}