Skip to content

Cleanup PyBuffer a bit #1662

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 6, 2022
Merged

Cleanup PyBuffer a bit #1662

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
104 changes: 74 additions & 30 deletions src/embed_tests/TestPyBuffer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System;
using System.Runtime.CompilerServices;
using System.Text;
using NUnit.Framework;
using Python.Runtime;
Expand All @@ -24,48 +25,40 @@ public void Dispose()
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())
{
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);
}
byte[] managedArray = { (byte)' ' };
buf.Write(managedArray, 0, managedArray.Length, 1);
}

string result = pythonArray.InvokeMethod("decode", "utf-8".ToPython()).As<string>();
Assert.IsTrue(result == bufferTestString2);
}

[Test]
public void TestBufferRead()
{
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())
{
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);
}
managedArray[0] = (byte)' ';
buf.Read(managedArray, 1, managedArray.Length - 1, 1);
}

string result = new UTF8Encoding().GetString(managedArray);
Assert.IsTrue(result == " " + bufferTestString.Substring(1));
}

[Test]
Expand All @@ -77,5 +70,56 @@ public void ArrayHasBuffer()
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();
}
}
}
5 changes: 3 additions & 2 deletions src/runtime/bufferinterface.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
25 changes: 24 additions & 1 deletion src/runtime/finalizer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ public ErrorArgs(Exception error)

private ConcurrentQueue<PendingFinalization> _objQueue = new();
private readonly ConcurrentQueue<PendingFinalization> _derivedQueue = new();
private readonly ConcurrentQueue<Py_buffer> _bufferQueue = new();
private int _throttled;

#region FINALIZER_CHECK
Expand Down Expand Up @@ -165,6 +166,19 @@ internal void AddDerivedFinalizedObject(ref IntPtr derived, int run)
_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;
Expand All @@ -178,7 +192,7 @@ internal static void Shutdown()

internal nint DisposeAll()
{
if (_objQueue.IsEmpty && _derivedQueue.IsEmpty)
if (_objQueue.IsEmpty && _derivedQueue.IsEmpty && _bufferQueue.IsEmpty)
return 0;

nint collected = 0;
Expand Down Expand Up @@ -242,6 +256,15 @@ internal nint DisposeAll()

collected++;
}

while (!_bufferQueue.IsEmpty)
{
if (!_bufferQueue.TryDequeue(out var buffer))
continue;

Runtime.PyBuffer_Release(ref buffer);
collected++;
}
}
finally
{
Expand Down
68 changes: 41 additions & 27 deletions src/runtime/pybuffer.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,9 @@ public static long SizeFromFormat(string format)
{
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>
Expand All @@ -113,7 +115,7 @@ public IntPtr GetPointer(long[] indices)
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>
Expand All @@ -126,7 +128,7 @@ public void FromContiguous(IntPtr buf, long len, BufferOrderStyle fort)
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();
}

Expand Down Expand Up @@ -173,44 +175,60 @@ internal void FillInfo(BorrowedReference exporter, IntPtr buf, long len, bool _r
/// <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 (_view.ndim != 1)
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 ((long)_view.len > int.MaxValue)
throw new NotSupportedException("Python buffers bigger than int.MaxValue are not supported.");
if (count > buffer.Length)
if (buffer is null)
throw new ArgumentNullException(nameof(buffer));

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 (count > (int)_view.len)
if (checked(count + destinationOffset) > _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.");
if (!this.IsContiguous(BufferOrderStyle.C))
throw new NotImplementedException("Only continuous buffers are supported");

Marshal.Copy(buffer, offset, _view.buf, count);
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.");
if (_view.len.ToInt64() > int.MaxValue)
throw new NotSupportedException("Python buffers bigger than int.MaxValue 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 (buffer is null)
throw new ArgumentNullException(nameof(buffer));

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 + 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.");

int copylen = count < (int)_view.len ? count : (int)_view.len;
Marshal.Copy(_view.buf, buffer, offset, copylen);
return copylen;
Marshal.Copy(_view.buf + sourceOffset, buffer, destinationOffset, count);
}

private bool disposedValue = false; // To detect redundant calls
Expand Down Expand Up @@ -240,11 +258,7 @@ private void Dispose(bool disposing)

if (_view.obj != IntPtr.Zero)
{
Finalizer.Instance.AddFinalizedObject(ref _view.obj, _exporter.run
#if TRACE_ALLOC
, _exporter.Traceback
#endif
);
Finalizer.Instance.AddFinalizedBuffer(ref _view);
}

Dispose(false);
Expand Down
6 changes: 6 additions & 0 deletions src/runtime/pyobject.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,11 @@ public static PyObject FromManagedObject(object ob)

internal bool IsDisposed => rawPtr == IntPtr.Zero;

void CheckDisposed()
{
if (IsDisposed) throw new ObjectDisposedException(nameof(PyObject));
}

protected virtual void Dispose(bool disposing)
{
if (IsDisposed)
Expand Down Expand Up @@ -1114,6 +1119,7 @@ public override int GetHashCode()
/// </remarks>
public PyBuffer GetBuffer(PyBUF flags = PyBUF.SIMPLE)
{
CheckDisposed();
return new PyBuffer(this, flags);
}

Expand Down
15 changes: 12 additions & 3 deletions src/runtime/runtime.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1109,7 +1109,7 @@ internal static nint PyBuffer_SizeFromFormat(string format)
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);
Expand Down Expand Up @@ -1364,6 +1364,13 @@ internal static NewReference EmptyPyBytes()
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);
Expand Down Expand Up @@ -1992,7 +1999,7 @@ static Delegates()
// 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));
Expand Down Expand Up @@ -2052,6 +2059,7 @@ static Delegates()
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));
Expand Down Expand Up @@ -2264,7 +2272,7 @@ static Delegates()
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; }
Expand Down Expand Up @@ -2324,6 +2332,7 @@ static Delegates()
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; }
Expand Down