diff --git a/src/ImageSharp/ImageSharp.csproj b/src/ImageSharp/ImageSharp.csproj index 1062e4b3a3..1b05bce713 100644 --- a/src/ImageSharp/ImageSharp.csproj +++ b/src/ImageSharp/ImageSharp.csproj @@ -13,6 +13,7 @@ Image Resize Crop Gif Jpg Jpeg Bitmap Png Tga NetCore A new, fully featured, fully managed, cross-platform, 2D graphics API for .NET Debug;Release;Debug-InnerLoop;Release-InnerLoop + $(DefineConstants);MEMORY_SENTINEL $(DefineConstants);DEBUG diff --git a/src/ImageSharp/Memory/Allocators/Internals/UniformUnmanagedMemoryPool.Buffer{T}.cs b/src/ImageSharp/Memory/Allocators/Internals/UniformUnmanagedMemoryPool.Buffer{T}.cs index bc4dc5c820..31b6d14345 100644 --- a/src/ImageSharp/Memory/Allocators/Internals/UniformUnmanagedMemoryPool.Buffer{T}.cs +++ b/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(); + } + /// 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 on the stack, // while loosing the reference to Image (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; + } } } } diff --git a/src/ImageSharp/Memory/Allocators/Internals/UniformUnmanagedMemoryPool.cs b/src/ImageSharp/Memory/Allocators/Internals/UniformUnmanagedMemoryPool.cs index 3d5c0da2c3..d709937deb 100644 --- a/src/ImageSharp/Memory/Allocators/Internals/UniformUnmanagedMemoryPool.cs +++ b/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]; } } diff --git a/src/ImageSharp/Memory/Allocators/Internals/UnmanagedMemoryHandle.cs b/src/ImageSharp/Memory/Allocators/Internals/UnmanagedMemoryHandle.cs index 013189683f..2ccf36d768 100644 --- a/src/ImageSharp/Memory/Allocators/Internals/UnmanagedMemoryHandle.cs +++ b/src/ImageSharp/Memory/Allocators/Internals/UnmanagedMemoryHandle.cs @@ -56,6 +56,8 @@ namespace SixLabors.ImageSharp.Memory.Internals /// 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((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 remainder = new Span((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; } } diff --git a/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs b/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs index 5fc563d095..9ea8e41f43 100644 --- a/src/ImageSharp/Memory/Allocators/UniformUnmanagedMemoryPoolMemoryAllocator.cs +++ b/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(this.pool, array, (int)totalLength); if (options.Has(AllocationOptions.Clean)) { diff --git a/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.Owned.cs b/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.Owned.cs index c02119a38c..3aa88651cf 100644 --- a/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.Owned.cs +++ b/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.Owned.cs @@ -30,8 +30,8 @@ namespace SixLabors.ImageSharp.Memory this.View = new MemoryGroupView(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[pooledBuffers.Length]; for (int i = 0; i < pooledBuffers.Length - 1; i++) { pooledBuffers[i].AssignedToNewOwner(); result[i] = new UniformUnmanagedMemoryPool.Buffer(pool, pooledBuffers[i], bufferLength); + if (options.Has(AllocationOptions.Clean)) + { + result[i].Clear(); + } } result[result.Length - 1] = new UniformUnmanagedMemoryPool.Buffer(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)] diff --git a/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.cs b/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.cs index f3b48b2eac..5f8666271f 100644 --- a/src/ImageSharp/Memory/DiscontiguousBuffers/MemoryGroup{T}.cs +++ b/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 Wrap(params Memory[] source)