Implement Dispose for CData (#1416)

* Implement Dispose for CData

* Fix code for multiple Dispose calls scenario

Author: BCSharp

Src/IronPython.Modules/_ctypes/CData.cs

@@ -10,6 +10,7 @@
 using System.Buffers;
 using System.Collections.Generic;
 using System.Diagnostics;
+using System.Threading;
 
 using IronPython.Runtime;
 using IronPython.Runtime.Exceptions;
@@ -38,7 +39,7 @@ public static partial class CTypes {
         /// Base class for all ctypes interop types.
         /// </summary>
         [PythonType("_CData"), PythonHidden]
-        public abstract class CData : IBufferProtocol, IPythonBuffer {
+        public abstract class CData : IBufferProtocol, IDisposable {
             internal MemoryHolder MemHolder {
                 get => _memHolder ?? throw new InvalidOperationException($"{nameof(CData)} object not fully initialized.");
                 set {
@@ -47,6 +48,7 @@ internal MemoryHolder MemHolder {
                 }
             }
             private MemoryHolder? _memHolder;
+            private bool _disposed;
 
             // members: __setstate__,  __reduce__ _b_needsfree_ __ctypes_from_outparam__ __hash__ _objects _b_base_ __doc__
             protected CData() { }
@@ -108,25 +110,37 @@ internal void InitializeFromBufferCopy(object? data, int offset, int size) {
             internal virtual PythonTuple GetBufferInfo()
                 => PythonTuple.MakeTuple(NativeType.TypeFormat, 0, PythonTuple.EMPTY);
 
+            [PythonHidden]
+            public void Dispose() {
+                if (!_disposed) {
+                    _disposed = true;
+                    if (_numExports == 0) {
+                        MemoryHolder? holder = Interlocked.Exchange(ref _memHolder, null);
+                        holder?.Dispose();
+                    }
+                }
+            }
 
-            #region IBufferProtocol Members
-
-            IPythonBuffer IBufferProtocol.GetBuffer(BufferFlags flags) => this;
-
-            void IDisposable.Dispose() { } // TODO
-
-            object IPythonBuffer.Object => this;
-
-            unsafe ReadOnlySpan<byte> IPythonBuffer.AsReadOnlySpan()
-                => new Span<byte>(MemHolder.UnsafeAddress.ToPointer(), MemHolder.Size);
+            #region IBufferProtocol
 
-            unsafe Span<byte> IPythonBuffer.AsSpan()
-                => new Span<byte>(MemHolder.UnsafeAddress.ToPointer(), MemHolder.Size);
+            private int _numExports;
 
-            unsafe MemoryHandle IPythonBuffer.Pin()
-                => new MemoryHandle(MemHolder.UnsafeAddress.ToPointer());
+            IPythonBuffer IBufferProtocol.GetBuffer(BufferFlags flags) {
+                if (_disposed) throw new ObjectDisposedException(GetType().Name);
+                _ = MemHolder; // check if fully initialized
+                Interlocked.Increment(ref _numExports);
+                return new CDataView(this);
+            }
 
-            int IPythonBuffer.Offset => 0;
+            // May be executed concurremtly with Dispose(), GetBuffer(), or another ReleaseBuffer()
+            private void ReleaseBuffer() {
+                Debug.Assert(_numExports > 0);
+                int cnt = Interlocked.Decrement(ref _numExports);
+                if (cnt == 0 && _disposed) {
+                    MemoryHolder? holder = Interlocked.Exchange(ref _memHolder, null);
+                    holder?.Dispose();
+                }
+            }
 
             [PythonHidden]
             public virtual int ItemCount {
@@ -141,21 +155,53 @@ public virtual int ItemCount {
                 }
             }
 
-            string IPythonBuffer.Format => NativeType.TypeFormat;
-
             [PythonHidden]
             public virtual int ItemSize => NativeType.Size;
 
-            int IPythonBuffer.NumOfDims => Shape?.Count ?? (ItemCount > 1 ? 1 : 0);
-
-            bool IPythonBuffer.IsReadOnly => false;
-
             [PythonHidden]
             public virtual IReadOnlyList<int>? Shape => null;
 
-            IReadOnlyList<int>? IPythonBuffer.Strides => null;
 
-            IReadOnlyList<int>? IPythonBuffer.SubOffsets => null;
+            private sealed class CDataView : IPythonBuffer {
+                private CData? _cdata;
+                private CData CData => _cdata ?? throw new ObjectDisposedException(nameof(CDataView));
+
+                internal CDataView(CData cdata) => _cdata = cdata;
+
+                public void Dispose() {
+                    CData? cdata = Interlocked.Exchange(ref _cdata, null);
+                    cdata?.ReleaseBuffer();
+                }
+
+                public object Object => CData;
+
+                unsafe ReadOnlySpan<byte> IPythonBuffer.AsReadOnlySpan()
+                    => new Span<byte>(CData.MemHolder.UnsafeAddress.ToPointer(), CData.MemHolder.Size);
+
+                unsafe Span<byte> IPythonBuffer.AsSpan()
+                    => new Span<byte>(CData.MemHolder.UnsafeAddress.ToPointer(), CData.MemHolder.Size);
+
+                unsafe MemoryHandle IPythonBuffer.Pin()
+                    => new MemoryHandle(CData.MemHolder.UnsafeAddress.ToPointer());
+
+                public int Offset => 0;
+
+                public int ItemCount => CData.ItemCount;
+
+                public string Format => CData.NativeType.TypeFormat;
+
+                public int ItemSize => CData.ItemSize;
+
+                public int NumOfDims => Shape?.Count ?? (ItemCount > 1 ? 1 : 0);
+
+                public bool IsReadOnly => false;
+
+                public IReadOnlyList<int>? Shape => CData.Shape;
+
+                IReadOnlyList<int>? IPythonBuffer.Strides => null;
+
+                IReadOnlyList<int>? IPythonBuffer.SubOffsets => null;
+            }
 
             #endregion
         }

Src/IronPython.Modules/_ctypes/MemoryHolder.cs

@@ -29,12 +29,11 @@ internal sealed class MemoryHolder : CriticalFinalizerObject, IDisposable {
         private readonly int _size;
         private readonly IPythonBuffer? _buffer;
         private readonly MemoryHandle _handle;
+        private readonly MemoryHolder? _parent;
+        private bool _disposeRequested;
         private bool _disposed;
+        private int _numChildren;
         private PythonDictionary? _objects;
-#pragma warning disable IDE0052 // Remove unread private members
-        // Field not accessed but keeps a reference to the parent holder preventing it from being garbage-collected.
-        private readonly MemoryHolder? _parent;
-#pragma warning restore IDE0052 // Remove unread private members
 
         /// <summary>
         /// Creates a new MemoryHolder and allocates a buffer of the specified size.
@@ -69,12 +68,12 @@ public MemoryHolder(IntPtr data, int size) {
         /// <summary>
         /// Creates a new MemoryHolder at the specified address which will keep alive the 
         /// parent memory holder.
-        /// TODO: Dispose() usage? Reference-count the parent?
         /// </summary>
         public MemoryHolder(IntPtr data, int size, MemoryHolder parent) {
             GC.SuppressFinalize(this);
             _data = data;
             _parent = parent;
+            parent._numChildren++;
             _objects = parent._objects;
             _size = size;
         }
@@ -86,9 +85,14 @@ public MemoryHolder(IntPtr data, int size, MemoryHolder parent) {
         public MemoryHolder(IPythonBuffer buffer, int offset, int size) {
             if (buffer.IsReadOnly) throw new ArgumentException("Buffer must be writable.");
             if (!buffer.IsCContiguous()) throw new ArgumentException("Buffer must be c-contiguous.");
-            if (size < 0) throw new ArgumentOutOfRangeException(nameof(size), size, $"Non-negative number required.");
-            if (offset < 0) throw new ArgumentOutOfRangeException(nameof(offset), offset, $"Non-negative number required.");
-            if (size > buffer.NumBytes() - offset) throw new ArgumentException($"Requested memory block exceeds buffer boundaries.");
+            if (size < 0) throw new ArgumentOutOfRangeException(nameof(size), size, "Non-negative number required.");
+            if (offset < 0) throw new ArgumentOutOfRangeException(nameof(offset), offset, "Non-negative number required.");
+
+            int bufSize = buffer.NumBytes();
+            ReadOnlySpan<byte> memblock = buffer.AsSpan();
+            if (memblock.Length != bufSize) new ArgumentException("Invalid buffer.");
+            if (size > bufSize - offset) throw new ArgumentException("Requested memory block exceeds buffer boundaries.");
+
 
             _buffer = buffer;
             _handle = buffer.Pin();
@@ -320,15 +324,23 @@ public void CopyTo(MemoryHolder/*!*/ destAddress, int writeOffset, int size) {
             GC.KeepAlive(this);
         }
 
+        private void ReleaseChild() {
+            Debug.Assert(_numChildren >= 0);
+            _numChildren--;
+            if (_disposeRequested) Dispose();
+        }
+
         public void Dispose() {
-            if (!_disposed) {
+            _disposeRequested = true;
+            if (!_disposed && _numChildren == 0) {
                 _disposed = true;
 
                 if (_ownsData) {
                     Marshal.FreeHGlobal(_data);
                 }
                 _handle.Dispose();
                 _buffer?.Dispose();
+                _parent?.ReleaseChild();
 
                 GC.SuppressFinalize(this);
             }
@@ -344,6 +356,7 @@ public void Dispose() {
                 }
                 try { _handle.Dispose(); } catch { /*ignore*/ }
                 try { _buffer?.Dispose(); } catch { /*ignore*/ }
+                try { _parent?.ReleaseChild(); } catch { /*ignore*/ }
             }
         }
     }