cleanup PyBuffer a bit:

- better finalizer, that actually calls PyBuffer_Release
- improved input parameter handling for common routines
- added support for copying data to/from large Python buffers

fixes https://github.com/pythonnet/pythonnet/issues/1556

src/embed_tests/TestPyBuffer.cs

@@ -1,4 +1,5 @@
 using System;
+using System.Runtime.CompilerServices;
 using System.Text;
 using NUnit.Framework;
 using Python.Runtime;
@@ -24,24 +25,20 @@ namespace Python.EmbeddingTest {
         public void TestBufferWrite()
         {
             string bufferTestString = "hello world! !$%&/()=?";
+            string bufferTestString2 = "h llo world! !$%&/()=?";
 
-            using (Py.GIL())
+            using var _ = Py.GIL();
+
+            using var pythonArray = ByteArrayFromAsciiString(bufferTestString);
+
+            using (PyBuffer buf = pythonArray.GetBuffer(PyBUF.WRITABLE))
             {
-                using (var scope = Py.CreateScope())
+                byte[] managedArray = { (byte)' ' };
-                {
+                buf.Write(managedArray, 0, managedArray.Length, 1);
-                    scope.Exec($"arr = bytearray({bufferTestString.Length})");
-                    PyObject pythonArray = scope.Get("arr");
-                    byte[] managedArray = new UTF8Encoding().GetBytes(bufferTestString);
-
-                    using (PyBuffer buf = pythonArray.GetBuffer())
-                    {
-                        buf.Write(managedArray, 0, managedArray.Length);
-                    }
-
-                    string result = scope.Eval("arr.decode('utf-8')").ToString();
-                    Assert.IsTrue(result == bufferTestString);
-                }
             }
+
+            string result = pythonArray.InvokeMethod("decode", "utf-8".ToPython()).As<string>();
+            Assert.IsTrue(result == bufferTestString2);
         }
 
         [Test]
@@ -49,23 +46,19 @@ namespace Python.EmbeddingTest {
         {
             string bufferTestString = "hello world! !$%&/()=?";
 
-            using (Py.GIL())
+            using var _ = Py.GIL();
+
+            using var pythonArray = ByteArrayFromAsciiString(bufferTestString);
+            byte[] managedArray = new byte[bufferTestString.Length];
+
+            using (PyBuffer buf = pythonArray.GetBuffer())
             {
-                using (var scope = Py.CreateScope())
+                managedArray[0] = (byte)' ';
-                {
+                buf.Read(managedArray, 1, managedArray.Length - 1, 1);
-                    scope.Exec($"arr = b'{bufferTestString}'");
-                    PyObject pythonArray = scope.Get("arr");
-                    byte[] managedArray = new byte[bufferTestString.Length];
-
-                    using (PyBuffer buf = pythonArray.GetBuffer())
-                    {
-                        buf.Read(managedArray, 0, managedArray.Length);
-                    }
-
-                    string result = new UTF8Encoding().GetString(managedArray);
-                    Assert.IsTrue(result == bufferTestString);
-                }
             }
+
+            string result = new UTF8Encoding().GetString(managedArray);
+            Assert.IsTrue(result == " " + bufferTestString.Substring(1));
         }
 
         [Test]
@@ -77,5 +70,56 @@ namespace Python.EmbeddingTest {
             Assert.AreEqual(1, mem[(0, 0).ToPython()].As<int>());
             Assert.AreEqual(array[1,0], mem[(1, 0).ToPython()].As<int>());
         }
+
+        [Test]
+        public void RefCount()
+        {
+            using var _ = Py.GIL();
+            using var arr = ByteArrayFromAsciiString("hello world! !$%&/()=?");
+
+            Assert.AreEqual(1, arr.Refcount);
+
+            using (PyBuffer buf = arr.GetBuffer())
+            {
+                Assert.AreEqual(2, arr.Refcount);
+            }
+
+            Assert.AreEqual(1, arr.Refcount);
+        }
+
+        [Test]
+        public void Finalization()
+        {
+            if (Type.GetType("Mono.Runtime") is not null)
+            {
+                Assert.Inconclusive("test unreliable in Mono");
+                return;
+            }
+
+            using var _ = Py.GIL();
+            using var arr = ByteArrayFromAsciiString("hello world! !$%&/()=?");
+
+            Assert.AreEqual(1, arr.Refcount);
+
+            MakeBufAndLeak(arr);
+
+            GC.Collect();
+            GC.WaitForPendingFinalizers();
+            Finalizer.Instance.Collect();
+
+            Assert.AreEqual(1, arr.Refcount);
+        }
+
+        [MethodImpl(MethodImplOptions.NoInlining)]
+        static void MakeBufAndLeak(PyObject bufProvider)
+        {
+            PyBuffer buf = bufProvider.GetBuffer();
+        }
+
+        static PyObject ByteArrayFromAsciiString(string str)
+        {
+            using var scope = Py.CreateScope();
+            return Runtime.Runtime.PyByteArray_FromStringAndSize(str).MoveToPyObject();
+        }
     }
 }

src/runtime/bufferinterface.cs

@@ -8,10 +8,11 @@ namespace Python.Runtime
     internal struct Py_buffer {
         public IntPtr buf;
         public IntPtr obj;        /* owned reference */
+        /// <summary>Buffer size in bytes</summary>
         [MarshalAs(UnmanagedType.SysInt)]
-        public IntPtr len;
+        public nint len;
         [MarshalAs(UnmanagedType.SysInt)]
-        public IntPtr itemsize;  /* This is Py_ssize_t so it can be
+        public nint itemsize;  /* This is Py_ssize_t so it can be
                              pointed to by strides in simple case.*/
         [MarshalAs(UnmanagedType.Bool)]
         public bool _readonly;

src/runtime/finalizer.cs

@@ -43,6 +43,7 @@ namespace Python.Runtime
 
         private ConcurrentQueue<PendingFinalization> _objQueue = new();
         private readonly ConcurrentQueue<PendingFinalization> _derivedQueue = new();
+        private readonly ConcurrentQueue<Py_buffer> _bufferQueue = new();
         private int _throttled;
 
         #region FINALIZER_CHECK
@@ -165,6 +166,19 @@ namespace Python.Runtime
             _derivedQueue.Enqueue(pending);
         }
 
+        internal void AddFinalizedBuffer(ref Py_buffer buffer)
+        {
+            if (buffer.obj == IntPtr.Zero)
+                throw new ArgumentNullException(nameof(buffer));
+
+            if (!Enable)
+                return;
+
+            var pending = buffer;
+            buffer = default;
+            _bufferQueue.Enqueue(pending);
+        }
+
         internal static void Initialize()
         {
             Instance.started = true;
@@ -178,7 +192,7 @@ namespace Python.Runtime
 
         internal nint DisposeAll()
         {
-            if (_objQueue.IsEmpty && _derivedQueue.IsEmpty)
+            if (_objQueue.IsEmpty && _derivedQueue.IsEmpty && _bufferQueue.IsEmpty)
                 return 0;
 
             nint collected = 0;
@@ -242,6 +256,15 @@ namespace Python.Runtime
 
                         collected++;
                     }
+
+                    while (!_bufferQueue.IsEmpty)
+                    {
+                        if (!_bufferQueue.TryDequeue(out var buffer))
+                            continue;
+
+                        Runtime.PyBuffer_Release(ref buffer);
+                        collected++;
+                    }
                 }
                 finally
                 {

src/runtime/pybuffer.cs

@@ -89,7 +89,9 @@ namespace Python.Runtime
         {
             if (Runtime.PyVersion < new Version(3,9))
                 throw new NotSupportedException("SizeFromFormat requires at least Python 3.9");
-            return (long)Runtime.PyBuffer_SizeFromFormat(format);
+            nint result = Runtime.PyBuffer_SizeFromFormat(format);
+            if (result == -1) throw PythonException.ThrowLastAsClrException();
+            return result;
         }
 
         /// <summary>
@@ -113,7 +115,7 @@ namespace Python.Runtime
                 throw new ObjectDisposedException(nameof(PyBuffer));
             if (Runtime.PyVersion < new Version(3, 7))
                 throw new NotSupportedException("GetPointer requires at least Python 3.7");
-            return Runtime.PyBuffer_GetPointer(ref _view, indices.Select(x => (IntPtr)x).ToArray());
+            return Runtime.PyBuffer_GetPointer(ref _view, indices.Select(x => checked((nint)x)).ToArray());
         }
 
         /// <summary>
@@ -126,7 +128,7 @@ namespace Python.Runtime
             if (Runtime.PyVersion < new Version(3, 7))
                 throw new NotSupportedException("FromContiguous requires at least Python 3.7");
 
-            if (Runtime.PyBuffer_FromContiguous(ref _view, buf, (IntPtr)len, OrderStyleToChar(fort, false)) < 0)
+            if (Runtime.PyBuffer_FromContiguous(ref _view, buf, checked((nint)len), OrderStyleToChar(fort, false)) < 0)
                 throw PythonException.ThrowLastAsClrException();
         }
 
@@ -173,44 +175,60 @@ namespace Python.Runtime
         /// <summary>
         /// Writes a managed byte array into the buffer of a python object. This can be used to pass data like images from managed to python.
         /// </summary>
-        public void Write(byte[] buffer, int offset, int count)
+        public void Write(byte[] buffer, int sourceOffset, int count, nint destinationOffset)
         {
             if (disposedValue)
                 throw new ObjectDisposedException(nameof(PyBuffer));
-            if (ReadOnly)
-                throw new InvalidOperationException("Buffer is read-only");
-            if ((long)_view.len > int.MaxValue)
-                throw new NotSupportedException("Python buffers bigger than int.MaxValue are not supported.");
-            if (count > buffer.Length)
-                throw new ArgumentOutOfRangeException("count", "Count is bigger than the buffer.");
-            if (count > (int)_view.len)
-                throw new ArgumentOutOfRangeException("count", "Count is bigger than the python buffer.");
             if (_view.ndim != 1)
-                throw new NotSupportedException("Multidimensional arrays, scalars and objects without a buffer are not supported.");
+                throw new NotImplementedException("Multidimensional arrays, scalars and objects without a buffer are not supported.");
             if (!this.IsContiguous(BufferOrderStyle.C))
                 throw new NotImplementedException("Only continuous buffers are supported");
+            if (ReadOnly)
+                throw new InvalidOperationException("Buffer is read-only");
+            if (buffer is null)
+                throw new ArgumentNullException(nameof(buffer));
 
-            Marshal.Copy(buffer, offset, _view.buf, count);
+            if (sourceOffset < 0)
+                throw new IndexOutOfRangeException($"{nameof(sourceOffset)} is negative");
+            if (destinationOffset < 0)
+                throw new IndexOutOfRangeException($"{nameof(destinationOffset)} is negative");
+            if (count < 0)
+                throw new ArgumentOutOfRangeException(nameof(count), count, "Value must be >= 0");
+
+            if (checked(count + sourceOffset) > buffer.Length)
+                throw new ArgumentOutOfRangeException("count", "Count is bigger than the buffer.");
+            if (checked(count + destinationOffset) > _view.len)
+                throw new ArgumentOutOfRangeException("count", "Count is bigger than the python buffer.");
+
+            Marshal.Copy(buffer, sourceOffset, _view.buf + destinationOffset, count);
         }
 
         /// <summary>
         /// Reads the buffer of a python object into a managed byte array. This can be used to pass data like images from python to managed.
         /// </summary>
-        public int Read(byte[] buffer, int offset, int count) {
+        public void Read(byte[] buffer, int destinationOffset, int count, nint sourceOffset) {
             if (disposedValue)
                 throw new ObjectDisposedException(nameof(PyBuffer));
-            if (count > buffer.Length)
-                throw new ArgumentOutOfRangeException("count", "Count is bigger than the buffer.");
             if (_view.ndim != 1)
-                throw new NotSupportedException("Multidimensional arrays, scalars and objects without a buffer are not supported.");
+                throw new NotImplementedException("Multidimensional arrays, scalars and objects without a buffer are not supported.");
-            if (_view.len.ToInt64() > int.MaxValue)
-                throw new NotSupportedException("Python buffers bigger than int.MaxValue are not supported.");
             if (!this.IsContiguous(BufferOrderStyle.C))
                 throw new NotImplementedException("Only continuous buffers are supported");
+            if (buffer is null)
+                throw new ArgumentNullException(nameof(buffer));
 
-            int copylen = count < (int)_view.len ? count : (int)_view.len;
+            if (sourceOffset < 0)
-            Marshal.Copy(_view.buf, buffer, offset, copylen);
+                throw new IndexOutOfRangeException($"{nameof(sourceOffset)} is negative");
-            return copylen;
+            if (destinationOffset < 0)
+                throw new IndexOutOfRangeException($"{nameof(destinationOffset)} is negative");
+            if (count < 0)
+                throw new ArgumentOutOfRangeException(nameof(count), count, "Value must be >= 0");
+
+            if (checked(count + destinationOffset) > buffer.Length)
+                throw new ArgumentOutOfRangeException("count", "Count is bigger than the buffer.");
+            if (checked(count + sourceOffset) > _view.len)
+                throw new ArgumentOutOfRangeException("count", "Count is bigger than the python buffer.");
+
+            Marshal.Copy(_view.buf + sourceOffset, buffer, destinationOffset, count);
         }
 
         private bool disposedValue = false; // To detect redundant calls
@@ -240,11 +258,7 @@ namespace Python.Runtime
 
             if (_view.obj != IntPtr.Zero)
             {
-                Finalizer.Instance.AddFinalizedObject(ref _view.obj, _exporter.run
+                Finalizer.Instance.AddFinalizedBuffer(ref _view);
-#if TRACE_ALLOC
-                    , _exporter.Traceback
-#endif
-                );
             }
 
             Dispose(false);

src/runtime/pyobject.cs

@@ -179,6 +179,11 @@ namespace Python.Runtime
 
         internal bool IsDisposed => rawPtr == IntPtr.Zero;
 
+        void CheckDisposed()
+        {
+            if (IsDisposed) throw new ObjectDisposedException(nameof(PyObject));
+        }
+
         protected virtual void Dispose(bool disposing)
         {
             if (IsDisposed)
@@ -1114,6 +1119,7 @@ namespace Python.Runtime
         /// </remarks>
         public PyBuffer GetBuffer(PyBUF flags = PyBUF.SIMPLE)
         {
+            CheckDisposed();
             return new PyBuffer(this, flags);
         }
 

src/runtime/runtime.cs

@@ -1107,7 +1107,7 @@ namespace Python.Runtime
         internal static int PyBuffer_IsContiguous(ref Py_buffer view, char order) => Delegates.PyBuffer_IsContiguous(ref view, order);
 
 
-        internal static IntPtr PyBuffer_GetPointer(ref Py_buffer view, IntPtr[] indices) => Delegates.PyBuffer_GetPointer(ref view, indices);
+        internal static IntPtr PyBuffer_GetPointer(ref Py_buffer view, nint[] indices) => Delegates.PyBuffer_GetPointer(ref view, indices);
 
 
         internal static int PyBuffer_FromContiguous(ref Py_buffer view, IntPtr buf, IntPtr len, char fort) => Delegates.PyBuffer_FromContiguous(ref view, buf, len, fort);
@@ -1362,6 +1362,13 @@ namespace Python.Runtime
             return Delegates.PyBytes_FromString((IntPtr)bytes);
         }
 
+        internal static NewReference PyByteArray_FromStringAndSize(IntPtr strPtr, nint len) => Delegates.PyByteArray_FromStringAndSize(strPtr, len);
+        internal static NewReference PyByteArray_FromStringAndSize(string s)
+        {
+            using var ptr = new StrPtr(s, Encoding.UTF8);
+            return PyByteArray_FromStringAndSize(ptr.RawPointer, checked((nint)ptr.ByteCount));
+        }
+
         internal static IntPtr PyBytes_AsString(BorrowedReference ob)
         {
             Debug.Assert(ob != null);
@@ -1977,7 +1984,7 @@ namespace Python.Runtime
                     // only in 3.9+
                 }
                 PyBuffer_IsContiguous = (delegate* unmanaged[Cdecl]<ref Py_buffer, char, int>)GetFunctionByName(nameof(PyBuffer_IsContiguous), GetUnmanagedDll(_PythonDll));
-                PyBuffer_GetPointer = (delegate* unmanaged[Cdecl]<ref Py_buffer, IntPtr[], IntPtr>)GetFunctionByName(nameof(PyBuffer_GetPointer), GetUnmanagedDll(_PythonDll));
+                PyBuffer_GetPointer = (delegate* unmanaged[Cdecl]<ref Py_buffer, nint[], IntPtr>)GetFunctionByName(nameof(PyBuffer_GetPointer), GetUnmanagedDll(_PythonDll));
                 PyBuffer_FromContiguous = (delegate* unmanaged[Cdecl]<ref Py_buffer, IntPtr, IntPtr, char, int>)GetFunctionByName(nameof(PyBuffer_FromContiguous), GetUnmanagedDll(_PythonDll));
                 PyBuffer_ToContiguous = (delegate* unmanaged[Cdecl]<IntPtr, ref Py_buffer, IntPtr, char, int>)GetFunctionByName(nameof(PyBuffer_ToContiguous), GetUnmanagedDll(_PythonDll));
                 PyBuffer_FillContiguousStrides = (delegate* unmanaged[Cdecl]<int, IntPtr, IntPtr, int, char, void>)GetFunctionByName(nameof(PyBuffer_FillContiguousStrides), GetUnmanagedDll(_PythonDll));
@@ -2037,6 +2044,7 @@ namespace Python.Runtime
                 PySequence_List = (delegate* unmanaged[Cdecl]<BorrowedReference, NewReference>)GetFunctionByName(nameof(PySequence_List), GetUnmanagedDll(_PythonDll));
                 PyBytes_AsString = (delegate* unmanaged[Cdecl]<BorrowedReference, IntPtr>)GetFunctionByName(nameof(PyBytes_AsString), GetUnmanagedDll(_PythonDll));
                 PyBytes_FromString = (delegate* unmanaged[Cdecl]<IntPtr, NewReference>)GetFunctionByName(nameof(PyBytes_FromString), GetUnmanagedDll(_PythonDll));
+                PyByteArray_FromStringAndSize = (delegate* unmanaged[Cdecl]<IntPtr, nint, NewReference>)GetFunctionByName(nameof(PyByteArray_FromStringAndSize), GetUnmanagedDll(_PythonDll));
                 PyBytes_Size = (delegate* unmanaged[Cdecl]<BorrowedReference, nint>)GetFunctionByName(nameof(PyBytes_Size), GetUnmanagedDll(_PythonDll));
                 PyUnicode_AsUTF8 = (delegate* unmanaged[Cdecl]<BorrowedReference, IntPtr>)GetFunctionByName(nameof(PyUnicode_AsUTF8), GetUnmanagedDll(_PythonDll));
                 PyUnicode_DecodeUTF16 = (delegate* unmanaged[Cdecl]<IntPtr, nint, IntPtr, IntPtr, NewReference>)GetFunctionByName(nameof(PyUnicode_DecodeUTF16), GetUnmanagedDll(_PythonDll));
@@ -2250,7 +2258,7 @@ namespace Python.Runtime
             internal static delegate* unmanaged[Cdecl]<ref Py_buffer, void> PyBuffer_Release { get; }
             internal static delegate* unmanaged[Cdecl]<StrPtr, nint> PyBuffer_SizeFromFormat { get; }
             internal static delegate* unmanaged[Cdecl]<ref Py_buffer, char, int> PyBuffer_IsContiguous { get; }
-            internal static delegate* unmanaged[Cdecl]<ref Py_buffer, IntPtr[], IntPtr> PyBuffer_GetPointer { get; }
+            internal static delegate* unmanaged[Cdecl]<ref Py_buffer, nint[], IntPtr> PyBuffer_GetPointer { get; }
             internal static delegate* unmanaged[Cdecl]<ref Py_buffer, IntPtr, IntPtr, char, int> PyBuffer_FromContiguous { get; }
             internal static delegate* unmanaged[Cdecl]<IntPtr, ref Py_buffer, IntPtr, char, int> PyBuffer_ToContiguous { get; }
             internal static delegate* unmanaged[Cdecl]<int, IntPtr, IntPtr, int, char, void> PyBuffer_FillContiguousStrides { get; }
@@ -2310,6 +2318,7 @@ namespace Python.Runtime
             internal static delegate* unmanaged[Cdecl]<BorrowedReference, NewReference> PySequence_List { get; }
             internal static delegate* unmanaged[Cdecl]<BorrowedReference, IntPtr> PyBytes_AsString { get; }
             internal static delegate* unmanaged[Cdecl]<IntPtr, NewReference> PyBytes_FromString { get; }
+            internal static delegate* unmanaged[Cdecl]<IntPtr, nint, NewReference> PyByteArray_FromStringAndSize { get; }
             internal static delegate* unmanaged[Cdecl]<BorrowedReference, nint> PyBytes_Size { get; }
             internal static delegate* unmanaged[Cdecl]<BorrowedReference, IntPtr> PyUnicode_AsUTF8 { get; }
             internal static delegate* unmanaged[Cdecl]<IntPtr, nint, IntPtr, IntPtr, NewReference> PyUnicode_DecodeUTF16 { get; }