MemoryAllocator fixes + debug mess

src/ImageSharp/ImageSharp.csproj

@@ -13,6 +13,7 @@
     <PackageTags>Image Resize Crop Gif Jpg Jpeg Bitmap Png Tga NetCore</PackageTags>
     <Description>A new, fully featured, fully managed, cross-platform, 2D graphics API for .NET</Description>
     <Configurations>Debug;Release;Debug-InnerLoop;Release-InnerLoop</Configurations>
+    <DefineConstants>$(DefineConstants);MEMORY_SENTINEL</DefineConstants>
     <DefineConstants Condition="'$(Configuration)' == 'Debug-InnerLoop'">$(DefineConstants);DEBUG</DefineConstants>
   </PropertyGroup>
 

src/ImageSharp/Memory/Allocators/Internals/UniformUnmanagedMemoryPool.Buffer{T}.cs

@@ -3,6 +3,7 @@
 
 using System;
 using System.Buffers;
+using System.Diagnostics;
 using System.Runtime.CompilerServices;
 
 namespace SixLabors.ImageSharp.Memory.Internals
@@ -14,9 +15,15 @@ namespace SixLabors.ImageSharp.Memory.Internals
         {
             private UniformUnmanagedMemoryPool pool;
 
+            private StackTrace _creatorStackTrace;
+
             public Buffer(UniformUnmanagedMemoryPool pool, UnmanagedMemoryHandle bufferHandle, int length)
-                : base(bufferHandle, length) =>
+                : base(bufferHandle, length)
+            {
                 this.pool = pool;
+                this._creatorStackTrace = new StackTrace();
+            }
+
 
             /// <inheritdoc />
             protected override void Dispose(bool disposing)
@@ -26,6 +33,11 @@ namespace SixLabors.ImageSharp.Memory.Internals
                     return;
                 }
 
+                if (this.BufferHandle.IsInvalid)
+                {
+                    throw new InvalidOperationException("The underlying handle has been disposed!");
+                }
+
                 this.VerifyMemorySentinel();
                 this.pool.Return(this.BufferHandle);
                 this.pool = null;
@@ -48,6 +60,8 @@ namespace SixLabors.ImageSharp.Memory.Internals
                 bufferHandle.AssignedToNewOwner();
             }
 
+            private bool disposedProperly;
+
             // A VERY poorly written user code holding a Span<TPixel> on the stack,
             // while loosing the reference to Image<TPixel> (or disposing it) may write to (now unrelated) pool buffer,
             // or cause memory corruption if the underlying UmnanagedMemoryHandle has been released.
@@ -66,6 +80,12 @@ namespace SixLabors.ImageSharp.Memory.Internals
                 }
 
                 base.Dispose(disposing);
+
+                if (disposing)
+                {
+                    GC.SuppressFinalize(this);
+                    this.disposedProperly = true;
+                }
             }
         }
     }

src/ImageSharp/Memory/Allocators/Internals/UniformUnmanagedMemoryPool.cs

@@ -81,10 +81,15 @@ namespace SixLabors.ImageSharp.Memory.Internals
                 buffer = UnmanagedMemoryHandle.Allocate(this.BufferLength);
             }
 
+            if (buffer.IsInvalid)
+            {
+                throw new InvalidOperationException("Rending disposed handle :O !!!");
+            }
+
             return buffer;
         }
 
-        public UnmanagedMemoryHandle[] Rent(int bufferCount, AllocationOptions allocationOptions = AllocationOptions.None)
+        public UnmanagedMemoryHandle[] Rent(int bufferCount)
         {
             UnmanagedMemoryHandle[] buffersLocal = this.buffers;
 
@@ -108,6 +113,11 @@ namespace SixLabors.ImageSharp.Memory.Internals
                 {
                     result[i] = buffersLocal[this.index];
                     buffersLocal[this.index++] = null;
+
+                    if (result[i].IsInvalid)
+                    {
+                        throw new InvalidOperationException("Renting disposed handle :O !!!");
+                    }
                 }
             }
 
@@ -117,11 +127,6 @@ namespace SixLabors.ImageSharp.Memory.Internals
                 {
                     result[i] = UnmanagedMemoryHandle.Allocate(this.BufferLength);
                 }
-
-                if (allocationOptions.Has(AllocationOptions.Clean))
-                {
-                    this.GetSpan(result[i]).Clear();
-                }
             }
 
             return result;
@@ -129,6 +134,11 @@ namespace SixLabors.ImageSharp.Memory.Internals
 
         public void Return(UnmanagedMemoryHandle buffer)
         {
+            if (buffer.IsInvalid)
+            {
+                throw new InvalidOperationException("Returning a disposed handle :O !!!");
+            }
+
             UnmanagedMemoryHandle[] buffersLocal = this.buffers;
             if (buffersLocal == null)
             {
@@ -183,6 +193,11 @@ namespace SixLabors.ImageSharp.Memory.Internals
 
                 for (int i = buffers.Length - 1; i >= 0; i--)
                 {
+                    if (buffers[i].IsInvalid)
+                    {
+                        throw new InvalidOperationException("Returning a disposed handle :O !!!");
+                    }
+
                     buffersLocal[--this.index] = buffers[i];
                 }
             }

src/ImageSharp/Memory/Allocators/Internals/UnmanagedMemoryHandle.cs

@@ -56,6 +56,8 @@ namespace SixLabors.ImageSharp.Memory.Internals
         /// <inheritdoc />
         public override bool IsInvalid => this.handle == IntPtr.Zero;
 
+        public StackTrace ReleaseHandleStackTrace { get; private set; }
+
         protected override bool ReleaseHandle()
         {
             if (this.IsInvalid)
@@ -63,6 +65,8 @@ namespace SixLabors.ImageSharp.Memory.Internals
                 return false;
             }
 
+            this.ReleaseHandleStackTrace = new StackTrace();
+
             Marshal.FreeHGlobal(this.handle);
             if (this.lengthInBytes > 0)
             {
@@ -91,19 +95,35 @@ namespace SixLabors.ImageSharp.Memory.Internals
         [Conditional("MEMORY_SENTINEL")]
         internal unsafe void InitMemorySentinel() => new Span<byte>((void*)this.handle, this.lengthInBytes + MemorySentinelPadding).Fill(42);
 
+        private volatile bool sentinelRunning;
+
         [Conditional("MEMORY_SENTINEL")]
         internal unsafe void VerifyMemorySentinel(int actualLengthInBytes)
         {
+            if (this.IsInvalid)
+            {
+                throw new InvalidOperationException("VerifyMemorySentinel on released handle!");
+            }
+
+            this.sentinelRunning = true;
+
             Span<byte> remainder =
                 new Span<byte>((void*)this.handle, this.lengthInBytes + MemorySentinelPadding)
                     .Slice(actualLengthInBytes);
             for (int i = 0; i < remainder.Length; i++)
             {
+                if (this.IsInvalid)
+                {
+                    throw new InvalidOperationException("mega wtf");
+                }
+
                 if (remainder[i] != 42)
                 {
                     throw new InvalidMemoryOperationException("Memory corruption detected!");
                 }
             }
+
+            this.sentinelRunning = false;
         }
 
         private static IntPtr AllocateHandle(int lengthInBytes)
@@ -149,9 +169,23 @@ namespace SixLabors.ImageSharp.Memory.Internals
         internal void Resurrect()
         {
             GC.SuppressFinalize(this);
+            // GC.ReRegisterForFinalize(this);
             this.resurrected = true;
+            this.reRegistered = false;
         }
 
+        // protected override void Dispose(bool disposing)
+        // {
+        //     if (!disposing)
+        //     {
+        //         GC.ReRegisterForFinalize(this);
+        //     }
+        //     else
+        //     {
+        //         base.Dispose(true);
+        //     }
+        // }
+
         internal void AssignedToNewOwner()
         {
             if (this.resurrected)
@@ -159,7 +193,10 @@ namespace SixLabors.ImageSharp.Memory.Internals
                 // The handle has been resurrected
                 GC.ReRegisterForFinalize(this);
                 this.resurrected = false;
+                this.reRegistered = true;
             }
         }
+
+        private bool reRegistered;
     }
 }

src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs

@@ -106,6 +106,8 @@ namespace SixLabors.ImageSharp.Memory
                     {
                         buffer.Clear();
                     }
+
+                    return buffer;
                 }
             }
 
@@ -131,6 +133,11 @@ namespace SixLabors.ImageSharp.Memory
                 UnmanagedMemoryHandle array = this.pool.Rent();
                 if (array != null)
                 {
+                    if (array.IsInvalid)
+                    {
+                        throw new InvalidOperationException("Rending disposed handle :O !!!");
+                    }
+
                     var buffer = new UniformUnmanagedMemoryPool.FinalizableBuffer<T>(this.pool, array, (int)totalLength);
                     if (options.Has(AllocationOptions.Clean))
                     {

src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.Owned.cs

@@ -30,8 +30,8 @@ namespace SixLabors.ImageSharp.Memory
                 this.View = new MemoryGroupView<T>(this);
             }
 
-            public Owned(UniformUnmanagedMemoryPool pool, UnmanagedMemoryHandle[] pooledArrays, int bufferLength, long totalLength, int sizeOfLastBuffer)
-                : this(CreateBuffers(pool, pooledArrays, bufferLength, sizeOfLastBuffer), bufferLength, totalLength, true)
+            public Owned(UniformUnmanagedMemoryPool pool, UnmanagedMemoryHandle[] pooledArrays, int bufferLength, long totalLength, int sizeOfLastBuffer, AllocationOptions options)
+                : this(CreateBuffers(pool, pooledArrays, bufferLength, sizeOfLastBuffer, options), bufferLength, totalLength, true)
             {
                 this.pooledHandles = pooledArrays;
                 this.unmanagedMemoryPool = pool;
@@ -66,13 +66,18 @@ namespace SixLabors.ImageSharp.Memory
                 UniformUnmanagedMemoryPool pool,
                 UnmanagedMemoryHandle[] pooledBuffers,
                 int bufferLength,
-                int sizeOfLastBuffer)
+                int sizeOfLastBuffer,
+                AllocationOptions options)
             {
                 var result = new IMemoryOwner<T>[pooledBuffers.Length];
                 for (int i = 0; i < pooledBuffers.Length - 1; i++)
                 {
                     pooledBuffers[i].AssignedToNewOwner();
                     result[i] = new UniformUnmanagedMemoryPool.Buffer<T>(pool, pooledBuffers[i], bufferLength);
+                    if (options.Has(AllocationOptions.Clean))
+                    {
+                        result[i].Clear();
+                    }
                 }
 
                 result[result.Length - 1] = new UniformUnmanagedMemoryPool.Buffer<T>(pool, pooledBuffers[pooledBuffers.Length - 1], sizeOfLastBuffer);
@@ -169,6 +174,11 @@ namespace SixLabors.ImageSharp.Memory
                 this.pooledArrays = null;
                 this.unmanagedMemoryPool = null;
                 this.pooledHandles = null;
+
+                if (disposing)
+                {
+                    GC.SuppressFinalize(this);
+                }
             }
 
             [MethodImpl(InliningOptions.ShortMethod)]

src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.cs

@@ -192,15 +192,14 @@ namespace SixLabors.ImageSharp.Memory
                 bufferCount++;
             }
 
-            UnmanagedMemoryHandle[] arrays = pool.Rent(bufferCount, options);
+            UnmanagedMemoryHandle[] arrays = pool.Rent(bufferCount);
 
             if (arrays == null)
             {
                 // Pool is full
                 return null;
             }
-
-            return new Owned(pool, arrays, bufferLength, totalLengthInElements, sizeOfLastBuffer);
+            return new Owned(pool, arrays, bufferLength, totalLengthInElements, sizeOfLastBuffer, options);
         }
 
         public static MemoryGroup<T> Wrap(params Memory<T>[] source)