When resizing, insured we didn't release old memory until after insert

BeefLibs/corlib/src/Collections/Dictionary.bf

@@ -12,6 +12,7 @@ namespace System.Collections
 	using System.Collections;
 	using System.Diagnostics;
 	using System.Diagnostics.Contracts;
+	using System.Threading;
 
 	public class Dictionary<TKey, TValue> :
 		ICollection<(TKey key, TValue value)>,
@@ -29,8 +30,8 @@ namespace System.Collections
 			public int_cosize mNext;        // Index of next entry, -1 if last
 		}
 
-		int_cosize* mBuckets ~ delete _;
-		Entry* mEntries ~ delete _;
+		int_cosize* mBuckets;
+		Entry* mEntries;
 		int_cosize mAllocSize;
 		int_cosize mCount;
 		int_cosize mFreeList;
@@ -49,6 +50,20 @@ namespace System.Collections
 			//TODO: this.comparer = comparer ?? EqualityComparer<TKey>.Default;
         }
 
+		public ~this()
+		{
+			if (mEntries != null)
+			{
+				var entries = mEntries;
+#if BF_ENABLE_REALTIME_LEAK_CHECK
+				// To avoid scanning items being deleted
+				mEntries = null;
+				Interlocked.Fence();
+#endif
+				Free(entries);
+			}
+		}
+
 		public int Count
 		{
 			get { return mCount - mFreeCount; }
@@ -123,7 +138,7 @@ namespace System.Collections
 		{
 			TKey* keyPtr;
 			TValue* valuePtr;
-			bool inserted = Insert(key, false, out keyPtr, out valuePtr);
+			bool inserted = Insert(key, false, out keyPtr, out valuePtr, null);
 			if (!inserted)
 				return false;
 			*keyPtr = key;
@@ -133,7 +148,32 @@ namespace System.Collections
 
 		public bool TryAdd(TKey key, out TKey* keyPtr, out TValue* valuePtr)
 		{
-            return Insert(key, false, out keyPtr, out valuePtr);
+            return Insert(key, false, out keyPtr, out valuePtr, null);
+		}
+
+		protected virtual (Entry*, int_cosize*) Alloc(int size)
+		{
+			int byteSize = size * (strideof(Entry) + sizeof(int_cosize));
+			uint8* allocPtr = new uint8[byteSize]*;
+			return ((Entry*)allocPtr, (int_cosize*)(allocPtr + size * strideof(Entry)));
+		}
+
+		protected virtual void Free(Entry* entryPtr)
+		{
+			delete (void*)entryPtr;
+		}
+
+		protected override void GCMarkMembers()
+		{
+			if (mEntries == null)
+				return;
+			let type = typeof(Entry);
+			if ((type.[Friend]mTypeFlags & .WantsMark) == 0)
+				return;
+		    for (int i < mCount)
+		    {
+		        GC.Mark_Unbound(mEntries[i]); 
+			}
 		}
 
 		public enum AddResult
@@ -145,8 +185,8 @@ namespace System.Collections
 		public AddResult TryAdd(TKey key)
 		{
 			TKey* keyPtr;
-			TValue* valuePtr;
-			if (Insert(key, false, out keyPtr, out valuePtr))
+			TValue* valuePtr;			
+			if (Insert(key, false, out keyPtr, out valuePtr, null))
 				return .Added(keyPtr, valuePtr);
 			return .Exists(keyPtr, valuePtr);
 		}
@@ -299,9 +339,8 @@ namespace System.Collections
 		private void Initialize(int capacity)
 		{
 			int_cosize size = GetPrimeish((int_cosize)capacity);
-			mBuckets = new int_cosize[size]*;
+			(mEntries, mBuckets) = Alloc(size);
 			for (int_cosize i < (int_cosize)size) mBuckets[i] = -1;
-			mEntries = new Entry[size]*;
 			mAllocSize = size;
 			mFreeList = -1;
 		}
@@ -328,6 +367,7 @@ namespace System.Collections
 				} 
 		    }
 			int_cosize index;
+			Entry* oldData = null;
 			if (mFreeCount > 0)
 			{
 				index = mFreeList;
@@ -338,7 +378,7 @@ namespace System.Collections
 			{
 				if (mCount == mAllocSize)
 				{
-					Resize();
+					oldData = Resize(false);
 					targetBucket = hashCode % (int_cosize)mAllocSize;
 				}
 				index = mCount;
@@ -350,17 +390,18 @@ namespace System.Collections
 			mEntries[index].mKey = key;
 			mEntries[index].mValue = value;
 			mBuckets[targetBucket] = index;
+			if (oldData != null)
+				Free(oldData);
 #if VERSION_DICTIONARY
 			mVersion++;
 #endif
 		}
 
-		private bool Insert(TKey key, bool add, out TKey* keyPtr, out TValue* valuePtr)
+		private bool Insert(TKey key, bool add, out TKey* keyPtr, out TValue* valuePtr, Entry** outOldData)
 		{
 			if (mBuckets == null) Initialize(0);
 			int32 hashCode = (int32)key.GetHashCode() & 0x7FFFFFFF;
 			int_cosize targetBucket = hashCode % (int_cosize)mAllocSize;
-
 			for (int_cosize i = mBuckets[targetBucket]; i >= 0; i = mEntries[i].mNext)
 			{
 				if (mEntries[i].mHashCode == hashCode && (mEntries[i].mKey == key))
@@ -371,10 +412,13 @@ namespace System.Collections
 					}
 					keyPtr = &mEntries[i].mKey;
 					valuePtr = &mEntries[i].mValue;
+					if (outOldData != null)
+						*outOldData = null;
 					return false;
 				} 
             }
 			int_cosize index;
+			Entry* oldData = null;
 			if (mFreeCount > 0)
 			{
 				index = mFreeList;
@@ -385,7 +429,7 @@ namespace System.Collections
 			{
 				if (mCount == mAllocSize)
 				{
-					Resize();
+					oldData = Resize(false);
 					targetBucket = hashCode % (int_cosize)mAllocSize;
 				}
 				index = mCount;
@@ -402,6 +446,10 @@ namespace System.Collections
 
 			keyPtr = &mEntries[index].mKey;
 			valuePtr = &mEntries[index].mValue;
+			if (outOldData != null)
+				*outOldData = oldData;
+			else
+				Free(oldData);
 			return true;
         }
 
@@ -418,17 +466,16 @@ namespace System.Collections
             return GetPrimeish(newSize);
 		}
 
-		private void Resize()
+		private Entry* Resize(bool autoFree)
 		{
-			Resize(ExpandSize(mCount), false);
+			return Resize(ExpandSize(mCount), false, autoFree);
 		}
 
-		private void Resize(int newSize, bool forceNewHashCodes)
+		private Entry* Resize(int newSize, bool forceNewHashCodes, bool autoFree)
 		{
 			Contract.Assert(newSize >= mAllocSize);
-			int_cosize* newBuckets = new int_cosize[newSize]*;
+			(var newEntries, var newBuckets) = Alloc(newSize);
 			for (int_cosize i = 0; i < newSize; i++) newBuckets[i] = -1;
-			Entry* newEntries = new Entry[newSize]*;
 			Internal.MemCpy(newEntries, mEntries, mCount * strideof(Entry), alignof(Entry));
 
 			if (forceNewHashCodes)
@@ -451,12 +498,17 @@ namespace System.Collections
 				}
 			}
 
-			delete mBuckets;
-			delete mEntries;
-
+			let oldPtr = mEntries;
 			mBuckets = newBuckets;
 			mEntries = newEntries;
 			mAllocSize = (int_cosize)newSize;
+
+			if (autoFree)
+			{
+				Free(oldPtr);
+				return null;
+			}
+			return oldPtr;
 		}
 
 		public bool Remove(TKey key)
@@ -875,7 +927,7 @@ namespace System.Collections
 		public struct KeyEnumerator : IEnumerator<TKey>, IResettable
 		{
 			private Dictionary<TKey, TValue> mDictionary;
-# if VERSION_DICTIONARY
+#if VERSION_DICTIONARY
 			private int32 mVersion;
 #endif
 			private int_cosize mIndex;

BeefLibs/corlib/src/Collections/List.bf

@@ -105,7 +105,8 @@ namespace System.Collections
 			if (IsDynAlloc)
 			{
 				var items = mItems;
-#if BF_ENABLE_REALTIME_LEAK_CHECK				
+#if BF_ENABLE_REALTIME_LEAK_CHECK
+				// To avoid scanning items being deleted
 				mItems = null;
 				Interlocked.Fence();
 #endif
@@ -138,41 +139,9 @@ namespace System.Collections
 				Debug.Assert((uint)value <= (uint)SizeFlags);
 				if (value != AllocSize)
 				{
-					if (value > 0)
-					{
-						T* newItems = Alloc(value);
-
-#if DBG
-						int typeId = typeof(T).GetTypeId();
-						if (typeId == sDebugTypeId)
-						{
-							Debug.WriteLine("Alloc {0} {1} {2}", scope Object[] { this, newItems, sDebugIdx } );
-							sDebugIdx++;
-						}
-#endif
-
-						if (mSize > 0)
-							Internal.MemCpy(newItems, mItems, mSize * strideof(T), alignof(T));
-
-						var oldItems = mItems;
-						mItems = newItems;
-						if (IsDynAlloc)
-						{
-#if BF_ENABLE_REALTIME_LEAK_CHECK
-							// We need to avoid scanning a deleted mItems
-							Interlocked.Fence();
-#endif						
-							Free(oldItems);
-						}
-						mAllocSizeAndFlags = (.)(value | DynAllocFlag);
-					}
-					else
-					{
-						if (IsDynAlloc)
-							Free(mItems);
-						mItems = null;
-						mAllocSizeAndFlags = 0;
-					}
+					T* oldAlloc = Realloc(value, true);
+					if (oldAlloc != null)
+						Free(oldAlloc);
 				}
 			}
 		}
@@ -259,39 +228,58 @@ namespace System.Collections
 			}
 		}
 
-		protected T* Alloc(int size)
+		protected virtual T* Alloc(int size)
 		{
 			return Internal.AllocRawArrayUnmarked<T>(size);
 		}
 
-		protected void Free(T* val)
+		protected virtual void Free(T* val)
 		{
 			delete (void*)val;
 		}
-		/*protected T[] Alloc(int size)
-		{
-			return new:this T[size];
-		}
 
-		protected void Free(Object obj)
+		T* Realloc(int newSize, bool autoFree)
 		{
-			delete:this obj;
-		}
+			T* oldAlloc = null;
+			if (newSize > 0)
+			{
+				T* newItems = Alloc(newSize);
 
-		protected virtual Object AllocObject(TypeInstance type, int size)
-		{
-			return Internal.ObjectAlloc(type, size);
-		}
+				if (mSize > 0)
+					Internal.MemCpy(newItems, mItems, mSize * strideof(T), alignof(T));
+				if (IsDynAlloc)
+					oldAlloc = mItems;
+				mItems = newItems;
+				mAllocSizeAndFlags = (.)(newSize | DynAllocFlag);
+			}
+			else
+			{
+				if (IsDynAlloc)
+					oldAlloc = mItems;
+				mItems = null;
+				mAllocSizeAndFlags = 0;
+			}
 
-		protected virtual void FreeObject(Object obj)
-		{
-			delete obj;
-		}*/
+			if ((autoFree) && (oldAlloc != null))
+			{
+				Free(oldAlloc);
+				return null;
+			}
+
+			return oldAlloc;
+		}
 
 		/// Adds an item to the back of the list.
 		public void Add(T item)
 		{
-			if (mSize == AllocSize) EnsureCapacity(mSize + 1);
+			if (mSize == AllocSize)
+			{
+				// We free after the insert to allow for inserting a ref to another list element
+				let oldPtr = EnsureCapacity(mSize + 1, false);
+				mItems[mSize++] = item;
+				Free(oldPtr);
+				return;
+			}
 			mItems[mSize++] = item;
 #if VERSION_LIST
 			mVersion++;
@@ -301,7 +289,14 @@ namespace System.Collections
 		/// Adds an item to the back of the list.
 		public void Add(Span<T> addSpan)
 		{
-			if (mSize == AllocSize) EnsureCapacity(mSize + addSpan.Length);
+			if (mSize == AllocSize)
+			{
+				let oldPtr = EnsureCapacity(mSize + addSpan.Length, false);
+				for (var val in ref addSpan)
+					mItems[mSize++] = val;
+				Free(oldPtr);
+				return;
+			}
 			for (var val in ref addSpan)
 				mItems[mSize++] = val;
 #if VERSION_LIST
@@ -312,7 +307,7 @@ namespace System.Collections
 		/// Returns a pointer to the start of the added uninitialized section
 		public T* GrowUnitialized(int addSize)
 		{
-			if (mSize + addSize > AllocSize) EnsureCapacity(mSize + addSize);
+			if (mSize + addSize > AllocSize) EnsureCapacity(mSize + addSize, true);
 			mSize += (int_cosize)addSize;
 #if VERSION_LIST
 			mVersion++;
@@ -365,7 +360,7 @@ namespace System.Collections
 
 		public void CopyTo(List<T> destList)
 		{
-			destList.EnsureCapacity(mSize);
+			destList.EnsureCapacity(mSize, true);
 			destList.mSize = mSize;
 			if (mSize > 0)
 				Internal.MemCpy(destList.mItems, mItems, mSize * strideof(T), alignof(T));
@@ -385,25 +380,23 @@ namespace System.Collections
 				array[i + arrayIndex] = mItems[i + index];
 		}
 
-		public void EnsureCapacity(int min)
+		public T* EnsureCapacity(int min, bool autoFree)
 		{
 			int allocSize = AllocSize;
-			if (allocSize < min)
-			{
-				int newCapacity = allocSize == 0 ? cDefaultCapacity : allocSize + allocSize / 2;
-				// If we overflow, try to set to max. The "< min" check after still still bump us up
-				// if necessary
-				if (newCapacity > SizeFlags) 
-					newCapacity = SizeFlags;
-				if (newCapacity < min)
-					newCapacity = min;
-				Capacity = newCapacity;
-			}
+			if (allocSize >= min)
+				return null;
+			
+			int_cosize newCapacity = (int_cosize)(allocSize == 0 ? cDefaultCapacity : allocSize * 2);
+			// Allow the list to grow to maximum possible capacity (~2G elements) before encountering overflow.
+			// Note that this check works even when mItems.Length overflowed thanks to the (uint) cast
+			//if ((uint)newCapacity > Array.MaxArrayLength) newCapacity = Array.MaxArrayLength;
+			if (newCapacity < min) newCapacity = (int_cosize)min;
+			return Realloc(newCapacity, autoFree);
 		}
 
 		public void Reserve(int size)
 		{
-			EnsureCapacity(size);
+			EnsureCapacity(size, true);
 		}
 
 		public Enumerator GetEnumerator()
@@ -453,7 +446,8 @@ namespace System.Collections
 
 		public void Insert(int index, T item)
 		{
-			if (mSize == AllocSize) EnsureCapacity(mSize + 1);
+			var item; // This creates a copy - required if item is a ref to an element
+			if (mSize == AllocSize) EnsureCapacity(mSize + 1, true);
 			if (index < mSize)
 			{
 				Internal.MemCpy(mItems + index + 1, mItems + index, (mSize - index) * strideof(T), alignof(T));
@@ -467,10 +461,12 @@ namespace System.Collections
 
 		public void Insert(int index, Span<T> items)
 		{
+			//TODO: Handle case where Span is a reference to ourselves
+
 			if (items.Length == 0)
 				return;
 			int addCount = items.Length;
-			if (mSize + addCount > AllocSize) EnsureCapacity(mSize + addCount);
+			if (mSize + addCount > AllocSize) EnsureCapacity(mSize + addCount, true);
 			if (index < mSize)
 			{
 				Internal.MemCpy(mItems + index + addCount, mItems + index, (mSize - index) * strideof(T), alignof(T));

BeefTools/BeefPerf/src/BPClient.bf

@@ -183,7 +183,7 @@ namespace BeefPerf
 				if (mStreamDataList.Capacity == 0)
 					mStreamDataList.Capacity = srcStreamDataList.Count;
 				else
-					mStreamDataList.EnsureCapacity(srcStreamDataList.Count);
+					mStreamDataList.EnsureCapacity(srcStreamDataList.Count, true);
 				mStreamDataList.GrowUnitialized(idx - (int32)mStreamDataList.Count + 1);
 			}
 

IDE/mintest/minlib/src/System/Collections/List.bf

@@ -140,33 +140,9 @@ namespace System.Collections
 				Debug.Assert((uint)value <= (uint)SizeFlags);
 				if (value != AllocSize)
 				{
-					if (value > 0)
-					{
-						T* newItems = Alloc(value);
-
-#if DBG
-						int typeId = typeof(T).GetTypeId();
-						if (typeId == sDebugTypeId)
-						{
-							Debug.WriteLine("Alloc {0} {1} {2}", scope Object[] { this, newItems, sDebugIdx } );
-							sDebugIdx++;
-						}
-#endif
-
-						if (mSize > 0)
-							Internal.MemCpy(newItems, mItems, mSize * strideof(T), alignof(T));
-						if (IsDynAlloc)
-							Free(mItems);
-						mItems = newItems;
-						mAllocSizeAndFlags = (.)(value | DynAllocFlag);
-					}
-					else
-					{
-						if (IsDynAlloc)
-							Free(mItems);
-						mItems = null;
-						mAllocSizeAndFlags = 0;
-					}
+					T* oldAlloc = Realloc(value, true);
+					if (oldAlloc != null)
+						Free(oldAlloc);
 				}
 			}
 		}
@@ -258,6 +234,46 @@ namespace System.Collections
 #endif
 		}
 
+		T* Realloc(int newSize, bool autoFree)
+		{
+			T* oldAlloc = null;
+ 			if (newSize > 0)
+			{
+				T* newItems = Alloc(newSize);
+
+#if DBG
+				int typeId = typeof(T).GetTypeId();
+				if (typeId == sDebugTypeId)
+				{
+					Debug.WriteLine("Alloc {0} {1} {2}", scope Object[] { this, newItems, sDebugIdx } );
+					sDebugIdx++;
+				}
+#endif
+
+				if (mSize > 0)
+					Internal.MemCpy(newItems, mItems, mSize * strideof(T), alignof(T));
+				if (IsDynAlloc)
+					oldAlloc = mItems;
+				mItems = newItems;
+				mAllocSizeAndFlags = (.)(newSize | DynAllocFlag);
+			}
+			else
+			{
+				if (IsDynAlloc)
+					oldAlloc = mItems;
+				mItems = null;
+				mAllocSizeAndFlags = 0;
+			}
+
+			if ((autoFree) && (oldAlloc != null))
+			{
+				Free(oldAlloc);
+				return null;
+			}
+
+			return oldAlloc;
+		}
+
 		protected void Free(T* val)
 		{
 			delete (void*)val;
@@ -285,12 +301,14 @@ namespace System.Collections
 		/// Adds an item to the back of the list.
 		public void Add(T item)
 		{
-			if (((int)Internal.UnsafeCastToPtr(this) & 0xFFFF) == 0x4BA0)
+			if (mSize == AllocSize)
 			{
-				NOP!();
+				// We free after the insert to allow for inserting a ref to another list element
+				let oldPtr = EnsureCapacity(mSize + 1, false);
+				mItems[mSize++] = item;
+				Free(oldPtr);
+				return;
 			}
-
-			if (mSize == AllocSize) EnsureCapacity(mSize + 1);
 			mItems[mSize++] = item;
 #if VERSION_LIST
 			mVersion++;
@@ -300,7 +318,7 @@ namespace System.Collections
 		/// Returns a pointer to the start of the added uninitialized section
 		public T* GrowUnitialized(int addSize)
 		{
-			if (mSize + addSize > AllocSize) EnsureCapacity(mSize + addSize);
+			if (mSize + addSize > AllocSize) EnsureCapacity(mSize + addSize, true);
 			mSize += (int_cosize)addSize;
 #if VERSION_LIST
 			mVersion++;
@@ -353,7 +371,7 @@ namespace System.Collections
 
 		public void CopyTo(List<T> destList)
 		{
-			destList.EnsureCapacity(mSize);
+			destList.EnsureCapacity(mSize, true);
 			destList.mSize = mSize;
 			if (mSize > 0)
 				Internal.MemCpy(destList.mItems, mItems, mSize * strideof(T), alignof(T));
@@ -366,23 +384,23 @@ namespace System.Collections
 				array[i + arrayIndex] = mItems[i];
 		}
 
-		public void EnsureCapacity(int min)
+		public T* EnsureCapacity(int min, bool autoFree)
 		{
 			int allocSize = AllocSize;
-			if (allocSize < min)
-			{
-				int_cosize newCapacity = (int_cosize)(allocSize == 0 ? cDefaultCapacity : allocSize * 2);
-				// Allow the list to grow to maximum possible capacity (~2G elements) before encountering overflow.
-				// Note that this check works even when mItems.Length overflowed thanks to the (uint) cast
-				//if ((uint)newCapacity > Array.MaxArrayLength) newCapacity = Array.MaxArrayLength;
-				if (newCapacity < min) newCapacity = (int_cosize)min;
-				Capacity = newCapacity;
-			}
+			if (allocSize >= min)
+				return null;
+			
+			int_cosize newCapacity = (int_cosize)(allocSize == 0 ? cDefaultCapacity : allocSize * 2);
+			// Allow the list to grow to maximum possible capacity (~2G elements) before encountering overflow.
+			// Note that this check works even when mItems.Length overflowed thanks to the (uint) cast
+			//if ((uint)newCapacity > Array.MaxArrayLength) newCapacity = Array.MaxArrayLength;
+			if (newCapacity < min) newCapacity = (int_cosize)min;
+			return Realloc(newCapacity, autoFree);
 		}
 
 		public void Reserve(int size)
 		{
-			EnsureCapacity(size);
+			EnsureCapacity(size, true);
 		}
 
 		public Enumerator GetEnumerator()
@@ -417,7 +435,14 @@ namespace System.Collections
 
 		public void Insert(int index, T item)
 		{
-			if (mSize == AllocSize) EnsureCapacity(mSize + 1);
+			if (mSize == AllocSize)
+			{
+				let oldPtr = EnsureCapacity(mSize + 1, false);
+				Internal.MemCpy(mItems + index + 1, mItems + index, (mSize - index) * strideof(T), alignof(T));
+				Free(oldPtr);
+				return;
+			}
+
 			if (index < mSize)
 			{
 				Internal.MemCpy(mItems + index + 1, mItems + index, (mSize - index) * strideof(T), alignof(T));