From 5eaedaeef07e26ca5af8d38ec258e33eaa7f1a64 Mon Sep 17 00:00:00 2001 From: Pavel Koneski Date: Sun, 2 Feb 2025 19:17:58 -0800 Subject: [PATCH 1/2] Avoid throw/catch exceptions when requesting optional buffer through `IBufferProtocol` --- src/core/IronPython.Modules/_ctypes/CData.cs | 2 +- src/core/IronPython.Modules/array.cs | 2 +- src/core/IronPython.Modules/mmap.cs | 11 +++++---- src/core/IronPython/Runtime/BufferProtocol.cs | 7 ++++-- src/core/IronPython/Runtime/ByteArray.cs | 2 +- src/core/IronPython/Runtime/Bytes.cs | 11 +++++---- .../IronPython/Runtime/ConversionWrappers.cs | 7 ++++-- src/core/IronPython/Runtime/MemoryView.cs | 23 ++++++++++++------- 8 files changed, 42 insertions(+), 23 deletions(-) diff --git a/src/core/IronPython.Modules/_ctypes/CData.cs b/src/core/IronPython.Modules/_ctypes/CData.cs index 147061c25..61e63bdc5 100644 --- a/src/core/IronPython.Modules/_ctypes/CData.cs +++ b/src/core/IronPython.Modules/_ctypes/CData.cs @@ -125,7 +125,7 @@ public void Dispose() { private int _numExports; - IPythonBuffer IBufferProtocol.GetBuffer(BufferFlags flags) { + IPythonBuffer IBufferProtocol.GetBuffer(BufferFlags flags, bool throwOnError) { if (_disposed) throw new ObjectDisposedException(GetType().Name); _ = MemHolder; // check if fully initialized Interlocked.Increment(ref _numExports); diff --git a/src/core/IronPython.Modules/array.cs b/src/core/IronPython.Modules/array.cs index 2bee03a5f..900b9d5b8 100644 --- a/src/core/IronPython.Modules/array.cs +++ b/src/core/IronPython.Modules/array.cs @@ -1037,7 +1037,7 @@ bool ICollection.Remove(object item) { #region IBufferProtocol Members - IPythonBuffer IBufferProtocol.GetBuffer(BufferFlags flags) { + IPythonBuffer IBufferProtocol.GetBuffer(BufferFlags flags, bool throwOnError) { return _data.GetBuffer(this, _typeCode.ToString(), flags); } diff --git a/src/core/IronPython.Modules/mmap.cs b/src/core/IronPython.Modules/mmap.cs index d33f997c3..94c31e3a5 100644 --- a/src/core/IronPython.Modules/mmap.cs +++ b/src/core/IronPython.Modules/mmap.cs @@ -1314,10 +1314,13 @@ void IWeakReferenceable.SetFinalizer(WeakRefTracker value) { #endregion - public IPythonBuffer GetBuffer(BufferFlags flags = BufferFlags.Simple) { - if (flags.HasFlag(BufferFlags.Writable) && IsReadOnly) - throw PythonOps.BufferError("Object is not writable."); - + public IPythonBuffer? GetBuffer(BufferFlags flags, bool throwOnError) { + if (flags.HasFlag(BufferFlags.Writable) && IsReadOnly) { + if (throwOnError) { + throw PythonOps.BufferError("Object is not writable."); + } + return null; + } return new MmapBuffer(this, flags); } diff --git a/src/core/IronPython/Runtime/BufferProtocol.cs b/src/core/IronPython/Runtime/BufferProtocol.cs index f537ae709..f4928201d 100644 --- a/src/core/IronPython/Runtime/BufferProtocol.cs +++ b/src/core/IronPython/Runtime/BufferProtocol.cs @@ -17,7 +17,7 @@ namespace IronPython.Runtime { /// Equivalent functionality of CPython's Buffer Protocol. /// public interface IBufferProtocol { - IPythonBuffer GetBuffer(BufferFlags flags = BufferFlags.Simple); + IPythonBuffer? GetBuffer(BufferFlags flags, bool throwOnError); } /// @@ -120,9 +120,12 @@ public enum BufferFlags { } internal static class BufferProtocolExtensions { + internal static IPythonBuffer GetBuffer(this IBufferProtocol bufferProtocol, BufferFlags flags = BufferFlags.Simple) + => bufferProtocol.GetBuffer(flags, throwOnError: true) ?? throw new BufferException("Buffer type not supported"); + internal static IPythonBuffer? GetBufferNoThrow(this IBufferProtocol bufferProtocol, BufferFlags flags = BufferFlags.Simple) { try { - return bufferProtocol.GetBuffer(flags); + return bufferProtocol.GetBuffer(flags, throwOnError: false); } catch (BufferException) { return null; } diff --git a/src/core/IronPython/Runtime/ByteArray.cs b/src/core/IronPython/Runtime/ByteArray.cs index 613b1400a..e541445be 100644 --- a/src/core/IronPython/Runtime/ByteArray.cs +++ b/src/core/IronPython/Runtime/ByteArray.cs @@ -1584,7 +1584,7 @@ private bool Equals(ReadOnlySpan other) { #region IBufferProtocol Members - IPythonBuffer IBufferProtocol.GetBuffer(BufferFlags flags) { + IPythonBuffer IBufferProtocol.GetBuffer(BufferFlags flags, bool throwOnError) { return _bytes.GetBuffer(this, "B", flags); } diff --git a/src/core/IronPython/Runtime/Bytes.cs b/src/core/IronPython/Runtime/Bytes.cs index 3726647d4..094d70219 100644 --- a/src/core/IronPython/Runtime/Bytes.cs +++ b/src/core/IronPython/Runtime/Bytes.cs @@ -1241,10 +1241,13 @@ Expression IExpressionSerializable.CreateExpression() { #region IBufferProtocol Support - IPythonBuffer IBufferProtocol.GetBuffer(BufferFlags flags) { - if (flags.HasFlag(BufferFlags.Writable)) - throw PythonOps.BufferError("Object is not writable."); - + IPythonBuffer? IBufferProtocol.GetBuffer(BufferFlags flags, bool throwOnError) { + if (flags.HasFlag(BufferFlags.Writable)) { + if (throwOnError) { + throw PythonOps.TypeError("bytes object is not writable"); + } + return null; + } return new BytesView(this, flags); } diff --git a/src/core/IronPython/Runtime/ConversionWrappers.cs b/src/core/IronPython/Runtime/ConversionWrappers.cs index 2a61dfd85..819e3f66e 100644 --- a/src/core/IronPython/Runtime/ConversionWrappers.cs +++ b/src/core/IronPython/Runtime/ConversionWrappers.cs @@ -436,13 +436,16 @@ public MemoryBufferProtocolWrapper(Memory memory) { _memory = memory; } - public IPythonBuffer GetBuffer(BufferFlags flags) { + public IPythonBuffer? GetBuffer(BufferFlags flags, bool throwOnError) { if (_memory.HasValue) { return new MemoryBufferWrapper(_memory.Value, flags); } if (flags.HasFlag(BufferFlags.Writable)) { - throw Operations.PythonOps.BufferError("ReadOnlyMemory is not writable."); + if (throwOnError) { + throw Operations.PythonOps.BufferError("ReadOnlyMemory is not writable."); + } + return null; } return new MemoryBufferWrapper(_rom, flags); diff --git a/src/core/IronPython/Runtime/MemoryView.cs b/src/core/IronPython/Runtime/MemoryView.cs index ffd9bbaab..94d8257b8 100644 --- a/src/core/IronPython/Runtime/MemoryView.cs +++ b/src/core/IronPython/Runtime/MemoryView.cs @@ -963,32 +963,39 @@ void IWeakReferenceable.SetFinalizer(WeakRefTracker value) { #region IBufferProtocol Members - IPythonBuffer IBufferProtocol.GetBuffer(BufferFlags flags) { + IPythonBuffer? IBufferProtocol.GetBuffer(BufferFlags flags, bool throwOnError) { CheckBuffer(); if (flags.HasFlag(BufferFlags.Writable) && _isReadOnly) - throw PythonOps.BufferError("memoryview: underlying buffer is not writable"); + return ReportError("memoryview: underlying buffer is not writable"); if (flags.HasFlag(BufferFlags.CContiguous) && !_isCContig) - throw PythonOps.BufferError("memoryview: underlying buffer is not C-contiguous"); + return ReportError("memoryview: underlying buffer is not C-contiguous"); if (flags.HasFlag(BufferFlags.FContiguous) && !_isFContig) - throw PythonOps.BufferError("memoryview: underlying buffer is not Fortran contiguous"); + return ReportError("memoryview: underlying buffer is not Fortran contiguous"); if (flags.HasFlag(BufferFlags.AnyContiguous) && !_isCContig && !_isFContig) - throw PythonOps.BufferError("memoryview: underlying buffer is not contiguous"); + return ReportError("memoryview: underlying buffer is not contiguous"); // TODO: Support for suboffsets //if (!flags.HasFlag(!BufferFlags.Indirect) && _suboffsets != null) - // throw PythonOps.BufferError("memoryview: underlying buffer requires suboffsets"); + // return ReportError("memoryview: underlying buffer requires suboffsets"); if (!flags.HasFlag(BufferFlags.Strides) && !_isCContig) - throw PythonOps.BufferError("memoryview: underlying buffer is not C-contiguous"); + return ReportError("memoryview: underlying buffer is not C-contiguous"); if (!flags.HasFlag(BufferFlags.ND) && flags.HasFlag(BufferFlags.Format)) - throw PythonOps.BufferError("memoryview: cannot cast to unsigned bytes if the format flag is present"); + return ReportError("memoryview: cannot cast to unsigned bytes if the format flag is present"); return new MemoryView(this, flags); + + IPythonBuffer? ReportError(string msg) { + if (throwOnError) { + throw PythonOps.BufferError(msg); + } + return null; + } } #endregion From cd988619b70348e480dfd79f3668c64307162a99 Mon Sep 17 00:00:00 2001 From: Pavel Koneski Date: Mon, 3 Feb 2025 21:18:22 -0800 Subject: [PATCH 2/2] Update after review --- src/core/IronPython/Runtime/BufferProtocol.cs | 30 +++++++++++++++++-- src/core/IronPython/Runtime/Bytes.cs | 2 +- 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/src/core/IronPython/Runtime/BufferProtocol.cs b/src/core/IronPython/Runtime/BufferProtocol.cs index f4928201d..457d526df 100644 --- a/src/core/IronPython/Runtime/BufferProtocol.cs +++ b/src/core/IronPython/Runtime/BufferProtocol.cs @@ -8,6 +8,7 @@ using System.Buffers; using System.Collections.Generic; using System.Diagnostics; +using System.Diagnostics.CodeAnalysis; using System.Runtime.InteropServices; using IronPython.Runtime.Exceptions; @@ -17,6 +18,19 @@ namespace IronPython.Runtime { /// Equivalent functionality of CPython's Buffer Protocol. /// public interface IBufferProtocol { + /// + /// Exports the object's data as a buffer. + /// + /// + /// Flags specifying the type of buffer the consumer is prepared to deal with. + /// + /// + /// This parameter is advisory: if true, the method may throw BufferError instead of returning null if the buffer type is not supported, + /// with the exception's message providing more information. + /// + /// + /// An instance of if the requested buffer type is supported, or null if not. + /// IPythonBuffer? GetBuffer(BufferFlags flags, bool throwOnError); } @@ -119,8 +133,9 @@ public enum BufferFlags { #endregion } - internal static class BufferProtocolExtensions { - internal static IPythonBuffer GetBuffer(this IBufferProtocol bufferProtocol, BufferFlags flags = BufferFlags.Simple) + + public static class BufferProtocolExtensions { + public static IPythonBuffer GetBuffer(this IBufferProtocol bufferProtocol, BufferFlags flags = BufferFlags.Simple) => bufferProtocol.GetBuffer(flags, throwOnError: true) ?? throw new BufferException("Buffer type not supported"); internal static IPythonBuffer? GetBufferNoThrow(this IBufferProtocol bufferProtocol, BufferFlags flags = BufferFlags.Simple) { @@ -130,8 +145,19 @@ internal static IPythonBuffer GetBuffer(this IBufferProtocol bufferProtocol, Buf return null; } } + + internal static bool TryGetBuffer(this IBufferProtocol bufferProtocol, BufferFlags flags, [NotNullWhen(true)] out IPythonBuffer? buffer) { + try { + buffer = bufferProtocol.GetBuffer(flags, throwOnError: false); + return buffer is not null; + } catch (BufferException) { + buffer = null; + return false; + } + } } + /// /// Provides low-level read-write access to byte data of the underlying object. /// diff --git a/src/core/IronPython/Runtime/Bytes.cs b/src/core/IronPython/Runtime/Bytes.cs index 094d70219..2518d86f9 100644 --- a/src/core/IronPython/Runtime/Bytes.cs +++ b/src/core/IronPython/Runtime/Bytes.cs @@ -1244,7 +1244,7 @@ Expression IExpressionSerializable.CreateExpression() { IPythonBuffer? IBufferProtocol.GetBuffer(BufferFlags flags, bool throwOnError) { if (flags.HasFlag(BufferFlags.Writable)) { if (throwOnError) { - throw PythonOps.TypeError("bytes object is not writable"); + throw PythonOps.BufferError("bytes object is not writable"); } return null; }