From 663df73b856b312460fc316ac121e0692dd87141 Mon Sep 17 00:00:00 2001 From: Victor Milovanov Date: Mon, 4 May 2020 18:08:56 -0700 Subject: [PATCH 01/39] reworked PythonException to simplify memory management, and enable .NET interop New method ThrowLastAsClrException should be used everywhere instead of obsolete PythonException constructor. It automatically restores .NET exceptions, and applies codecs for Python exceptions. Traceback, PyVal and PyType are now stored and returned as PyObjects. PythonException now has InnerException set from its cause (e.g. __cause__ in Python, if any). PythonException.Restore no longer clears the exception instance. All helper methods were removed from public API surface. --- src/runtime/BorrowedReference.cs | 2 + src/runtime/NewReference.cs | 5 + src/runtime/converterextensions.cs | 2 + src/runtime/exceptions.cs | 5 +- src/runtime/managedtype.cs | 6 + src/runtime/pyobject.cs | 15 +- src/runtime/pythonengine.cs | 17 +- src/runtime/pythonexception.cs | 286 ++++++++++++++++++++--------- src/runtime/runtime.cs | 4 +- 9 files changed, 235 insertions(+), 107 deletions(-) diff --git a/src/runtime/BorrowedReference.cs b/src/runtime/BorrowedReference.cs index a3bf29056..a2af1c7bf 100644 --- a/src/runtime/BorrowedReference.cs +++ b/src/runtime/BorrowedReference.cs @@ -14,6 +14,8 @@ readonly ref struct BorrowedReference public IntPtr DangerousGetAddress() => this.IsNull ? throw new NullReferenceException() : this.pointer; + public static BorrowedReference Null => new BorrowedReference(); + /// /// Creates new instance of from raw pointer. Unsafe. /// diff --git a/src/runtime/NewReference.cs b/src/runtime/NewReference.cs index 6e66232d0..2e27f2b2b 100644 --- a/src/runtime/NewReference.cs +++ b/src/runtime/NewReference.cs @@ -28,6 +28,11 @@ public PyObject MoveToPyObject() return result; } /// + /// Returns wrapper around this reference, which now owns + /// the pointer. Sets the original reference to null, as it no longer owns it. + /// + public PyObject MoveToPyObjectOrNull() => this.IsNull() ? null : this.MoveToPyObject(); + /// /// Removes this reference to a Python object, and sets it to null. /// public void Dispose() diff --git a/src/runtime/converterextensions.cs b/src/runtime/converterextensions.cs index 667fc6f00..de3067f7a 100644 --- a/src/runtime/converterextensions.cs +++ b/src/runtime/converterextensions.cs @@ -110,6 +110,8 @@ static IPyObjectEncoder[] GetEncoders(Type type) #region Decoding static readonly ConcurrentDictionary pythonToClr = new ConcurrentDictionary(); + internal static bool TryDecode(BorrowedReference value, BorrowedReference type, Type targetType, out object result) + => TryDecode(value.DangerousGetAddress(), type.DangerousGetAddress(), targetType, out result); internal static bool TryDecode(IntPtr pyHandle, IntPtr pyType, Type targetType, out object result) { if (pyHandle == IntPtr.Zero) throw new ArgumentNullException(nameof(pyHandle)); diff --git a/src/runtime/exceptions.cs b/src/runtime/exceptions.cs index 31c367eb2..c7837f7b9 100644 --- a/src/runtime/exceptions.cs +++ b/src/runtime/exceptions.cs @@ -279,10 +279,7 @@ public static void SetError(Exception e) var pe = e as PythonException; if (pe != null) { - Runtime.XIncref(pe.PyType); - Runtime.XIncref(pe.PyValue); - Runtime.XIncref(pe.PyTB); - Runtime.PyErr_Restore(pe.PyType, pe.PyValue, pe.PyTB); + pe.Restore(); return; } diff --git a/src/runtime/managedtype.cs b/src/runtime/managedtype.cs index 3191da949..b5fa58042 100644 --- a/src/runtime/managedtype.cs +++ b/src/runtime/managedtype.cs @@ -15,6 +15,12 @@ internal abstract class ManagedType internal IntPtr tpHandle; // PyType * + /// + /// Given a Python object, return the associated managed object or null. + /// + internal static ManagedType GetManagedObject(BorrowedReference ob) + => GetManagedObject(ob.DangerousGetAddress()); + /// /// Given a Python object, return the associated managed object or null. /// diff --git a/src/runtime/pyobject.cs b/src/runtime/pyobject.cs index 699ebf873..74e31f7e5 100644 --- a/src/runtime/pyobject.cs +++ b/src/runtime/pyobject.cs @@ -31,7 +31,10 @@ public class PyObject : DynamicObject, IEnumerable, IPyDisposable protected internal IntPtr obj = IntPtr.Zero; - internal BorrowedReference Reference => new BorrowedReference(obj); + public static PyObject None => new PyObject(new BorrowedReference(Runtime.PyNone)); + internal BorrowedReference Reference => new BorrowedReference(this.obj); + internal NewReference MakeNewReference() + => NewReference.DangerousFromPointer(Runtime.SelfIncRef(this.obj)); /// /// PyObject Constructor @@ -125,6 +128,13 @@ public static PyObject FromManagedObject(object ob) return new PyObject(op); } + /// + /// Creates new from a nullable reference. + /// When is null, null is returned. + /// + internal static PyObject FromNullableReference(BorrowedReference reference) + => reference.IsNull ? null : new PyObject(reference); + /// /// AsManagedObject Method @@ -226,6 +236,9 @@ public IntPtr[] GetTrackedHandles() return new IntPtr[] { obj }; } + internal BorrowedReference GetPythonTypeReference() + => new BorrowedReference(Runtime.PyObject_TYPE(obj)); + /// /// GetPythonType Method /// diff --git a/src/runtime/pythonengine.cs b/src/runtime/pythonengine.cs index 11fc88cd4..18c4698b4 100644 --- a/src/runtime/pythonengine.cs +++ b/src/runtime/pythonengine.cs @@ -755,9 +755,9 @@ public static void With(PyObject obj, Action Body) // Behavior described here: // https://docs.python.org/2/reference/datamodel.html#with-statement-context-managers - IntPtr type = Runtime.PyNone; - IntPtr val = Runtime.PyNone; - IntPtr traceBack = Runtime.PyNone; + PyObject type = PyObject.None; + PyObject val = PyObject.None; + PyObject traceBack = PyObject.None; PythonException ex = null; try @@ -769,15 +769,12 @@ public static void With(PyObject obj, Action Body) catch (PythonException e) { ex = e; - type = ex.PyType.Coalesce(type); - val = ex.PyValue.Coalesce(val); - traceBack = ex.PyTB.Coalesce(traceBack); + type = ex.PyType ?? type; + val = ex.PyValue ?? val; + traceBack = ex.PyTB ?? traceBack; } - Runtime.XIncref(type); - Runtime.XIncref(val); - Runtime.XIncref(traceBack); - var exitResult = obj.InvokeMethod("__exit__", new PyObject(type), new PyObject(val), new PyObject(traceBack)); + var exitResult = obj.InvokeMethod("__exit__", type, val, traceBack); if (ex != null && !exitResult.IsTrue()) throw ex; } diff --git a/src/runtime/pythonexception.cs b/src/runtime/pythonexception.cs index 8efdccc91..aa4f4df9b 100644 --- a/src/runtime/pythonexception.cs +++ b/src/runtime/pythonexception.cs @@ -1,5 +1,6 @@ using System; using System.Runtime.CompilerServices; +using System.Runtime.ExceptionServices; using System.Text; namespace Python.Runtime @@ -8,78 +9,221 @@ namespace Python.Runtime /// Provides a managed interface to exceptions thrown by the Python /// runtime. /// - public class PythonException : System.Exception, IPyDisposable + public class PythonException : System.Exception, IDisposable { - private IntPtr _pyType = IntPtr.Zero; - private IntPtr _pyValue = IntPtr.Zero; - private IntPtr _pyTB = IntPtr.Zero; - private string _tb = ""; + private PyObject _type; + private PyObject _value; + private PyObject _pyTB; + private string _traceback = ""; private string _message = ""; private string _pythonTypeName = ""; private bool disposed = false; - private bool _finalized = false; + [Obsolete("Please, use ThrowLastAsClrException or FromPyErr instead")] public PythonException() { IntPtr gs = PythonEngine.AcquireLock(); - Runtime.PyErr_Fetch(out _pyType, out _pyValue, out _pyTB); - if (_pyType != IntPtr.Zero && _pyValue != IntPtr.Zero) + Runtime.PyErr_Fetch(out var type, out var value, out var traceback); + _type = type.MoveToPyObjectOrNull(); + _value = value.MoveToPyObjectOrNull(); + _pyTB = traceback.MoveToPyObjectOrNull(); + if (_type != null && _value != null) { - string type; string message; - Runtime.XIncref(_pyType); - using (var pyType = new PyObject(_pyType)) - using (PyObject pyTypeName = pyType.GetAttr("__name__")) + using (PyObject pyTypeName = _type.GetAttr("__name__")) { - type = pyTypeName.ToString(); + _pythonTypeName = pyTypeName.ToString(); } - _pythonTypeName = type; + message = _value.ToString(); + _message = _pythonTypeName + " : " + message; + } + if (_pyTB != null) + { + _traceback = TracebackToString(_pyTB); + } + PythonEngine.ReleaseLock(gs); + } + + private PythonException(BorrowedReference pyTypeHandle, + BorrowedReference pyValueHandle, + BorrowedReference pyTracebackHandle, + string message, string pythonTypeName, string traceback, + Exception innerException) + : base(message, innerException) + { + _type = PyObject.FromNullableReference(pyTypeHandle); + _value = PyObject.FromNullableReference(pyValueHandle); + _pyTB = PyObject.FromNullableReference(pyTracebackHandle); + _message = message; + _pythonTypeName = pythonTypeName ?? _pythonTypeName; + _traceback = traceback ?? _traceback; + } - Runtime.XIncref(_pyValue); - using (var pyValue = new PyObject(_pyValue)) + internal static Exception FromPyErr() + { + Runtime.PyErr_Fetch(out var type, out var value, out var traceback); + try + { + return FromPyErr( + typeHandle: type, + valueHandle: value, + tracebackHandle: traceback); + } + finally + { + type.Dispose(); + value.Dispose(); + traceback.Dispose(); + } + } + + internal static Exception FromPyErrOrNull() + { + Runtime.PyErr_Fetch(out var type, out var value, out var traceback); + try + { + if (value.IsNull() && type.IsNull() && traceback.IsNull()) { - message = pyValue.ToString(); + return null; } - _message = type + " : " + message; + + var result = FromPyErr(type, value, traceback); + return result; } - if (_pyTB != IntPtr.Zero) + finally + { + type.Dispose(); + value.Dispose(); + traceback.Dispose(); + } + } + + /// + /// Rethrows the last Python exception as corresponding CLR exception. + /// It is recommended to call this as throw ThrowLastAsClrException() + /// to assist control flow checks. + /// + internal static Exception ThrowLastAsClrException() + { + IntPtr gs = PythonEngine.AcquireLock(); + try { - using (PyObject tb_module = PythonEngine.ImportModule("traceback")) + Runtime.PyErr_Fetch(out var pyTypeHandle, out var pyValueHandle, out var pyTracebackHandle); + try { - Runtime.XIncref(_pyTB); - using (var pyTB = new PyObject(_pyTB)) + var clrObject = ManagedType.GetManagedObject(pyValueHandle) as CLRObject; + if (clrObject?.inst is Exception e) { - _tb = tb_module.InvokeMethod("format_tb", pyTB).ToString(); +#if NETSTANDARD + ExceptionDispatchInfo.Capture(e).Throw(); +#endif + throw e; } + + var result = FromPyErr(pyTypeHandle, pyValueHandle, pyTracebackHandle); + throw result; + } + finally + { + pyTypeHandle.Dispose(); + pyValueHandle.Dispose(); + pyTracebackHandle.Dispose(); } } - PythonEngine.ReleaseLock(gs); + finally + { + PythonEngine.ReleaseLock(gs); + } } - // Ensure that encapsulated Python objects are decref'ed appropriately - // when the managed exception wrapper is garbage-collected. + /// + /// Requires lock to be acquired elsewhere + /// + static Exception FromPyErr(BorrowedReference typeHandle, BorrowedReference valueHandle, BorrowedReference tracebackHandle) + { + Exception inner = null; + string pythonTypeName = null, msg = "", traceback = null; + + var clrObject = ManagedType.GetManagedObject(valueHandle) as CLRObject; + if (clrObject?.inst is Exception e) + { + return e; + } + + if (!typeHandle.IsNull && !valueHandle.IsNull) + { + if (PyObjectConversions.TryDecode(valueHandle, typeHandle, typeof(Exception), out object decoded) + && decoded is Exception decodedException) + { + return decodedException; + } + + string type; + string message; + using (var pyType = new PyObject(typeHandle)) + using (PyObject pyTypeName = pyType.GetAttr("__name__")) + { + type = pyTypeName.ToString(); + } + + pythonTypeName = type; + + using (var pyValue = new PyObject(valueHandle)) + { + message = pyValue.ToString(); + var cause = pyValue.GetAttr("__cause__", null); + if (cause != null && cause.Handle != Runtime.PyNone) + { + using (var innerTraceback = cause.GetAttr("__traceback__", null)) + { + inner = FromPyErr( + typeHandle: cause.GetPythonTypeReference(), + valueHandle: cause.Reference, + tracebackHandle: innerTraceback is null + ? BorrowedReference.Null + : innerTraceback.Reference); + } + } + } + msg = type + " : " + message; + } + if (!tracebackHandle.IsNull) + { + traceback = TracebackToString(new PyObject(tracebackHandle)); + } + + return new PythonException(typeHandle, valueHandle, tracebackHandle, + msg, pythonTypeName, traceback, inner); + } - ~PythonException() + static string TracebackToString(PyObject traceback) { - if (_finalized || disposed) + if (traceback is null) { - return; + throw new ArgumentNullException(nameof(traceback)); } - _finalized = true; - Finalizer.Instance.AddFinalizedObject(this); + + PyObject tracebackModule = PythonEngine.ImportModule("traceback"); + PyList stackLines = new PyList(tracebackModule.InvokeMethod("format_tb", traceback)); + stackLines.Reverse(); + var result = new StringBuilder(); + foreach (object stackLine in stackLines) + { + result.Append(stackLine); + } + return result.ToString(); } /// - /// Restores python error. + /// Restores python error. Clears this instance. /// public void Restore() { + if (this.disposed) throw new ObjectDisposedException(nameof(PythonException)); + IntPtr gs = PythonEngine.AcquireLock(); - Runtime.PyErr_Restore(_pyType, _pyValue, _pyTB); - _pyType = IntPtr.Zero; - _pyValue = IntPtr.Zero; - _pyTB = IntPtr.Zero; + Runtime.PyErr_Restore(_type.MakeNewReference(), _value.MakeNewReference(), _pyTB.MakeNewReference()); PythonEngine.ReleaseLock(gs); } @@ -89,10 +233,7 @@ public void Restore() /// /// Returns the exception type as a Python object. /// - public IntPtr PyType - { - get { return _pyType; } - } + public PyObject PyType => _type; /// /// PyValue Property @@ -100,10 +241,7 @@ public IntPtr PyType /// /// Returns the exception value as a Python object. /// - public IntPtr PyValue - { - get { return _pyValue; } - } + public PyObject PyValue => _value; /// /// PyTB Property @@ -111,10 +249,7 @@ public IntPtr PyValue /// /// Returns the TraceBack as a Python object. /// - public IntPtr PyTB - { - get { return _pyTB; } - } + public PyObject PyTB => _pyTB; /// /// Message Property @@ -135,7 +270,7 @@ public override string Message /// public override string StackTrace { - get { return _tb + base.StackTrace; } + get { return _traceback + base.StackTrace; } } /// @@ -156,18 +291,12 @@ public string Format() IntPtr gs = PythonEngine.AcquireLock(); try { - if (_pyTB != IntPtr.Zero && _pyType != IntPtr.Zero && _pyValue != IntPtr.Zero) + if (_pyTB != null && _type != null && _value != null) { - Runtime.XIncref(_pyType); - Runtime.XIncref(_pyValue); - Runtime.XIncref(_pyTB); - using (PyObject pyType = new PyObject(_pyType)) - using (PyObject pyValue = new PyObject(_pyValue)) - using (PyObject pyTB = new PyObject(_pyTB)) using (PyObject tb_mod = PythonEngine.ImportModule("traceback")) { var buffer = new StringBuilder(); - var values = tb_mod.InvokeMethod("format_exception", pyType, pyValue, pyTB); + var values = tb_mod.InvokeMethod("format_exception", _type, _value, _pyTB); foreach (PyObject val in values) { buffer.Append(val.ToString()); @@ -200,39 +329,14 @@ public void Dispose() { if (!disposed) { - if (Runtime.Py_IsInitialized() > 0 && !Runtime.IsFinalizing) - { - IntPtr gs = PythonEngine.AcquireLock(); - if (_pyType != IntPtr.Zero) - { - Runtime.XDecref(_pyType); - _pyType= IntPtr.Zero; - } - - if (_pyValue != IntPtr.Zero) - { - Runtime.XDecref(_pyValue); - _pyValue = IntPtr.Zero; - } - - // XXX Do we ever get TraceBack? // - if (_pyTB != IntPtr.Zero) - { - Runtime.XDecref(_pyTB); - _pyTB = IntPtr.Zero; - } - PythonEngine.ReleaseLock(gs); - } + _type?.Dispose(); + _value?.Dispose(); + _pyTB?.Dispose(); GC.SuppressFinalize(this); disposed = true; } } - public IntPtr[] GetTrackedHandles() - { - return new IntPtr[] { _pyType, _pyValue, _pyTB }; - } - /// /// Matches Method /// @@ -240,24 +344,26 @@ public IntPtr[] GetTrackedHandles() /// Returns true if the Python exception type represented by the /// PythonException instance matches the given exception type. /// - public static bool Matches(IntPtr ob) + internal static bool Matches(IntPtr ob) { return Runtime.PyErr_ExceptionMatches(ob) != 0; } - public static void ThrowIfIsNull(IntPtr ob) + internal static IntPtr ThrowIfIsNull(IntPtr ob) { if (ob == IntPtr.Zero) { - throw new PythonException(); + throw ThrowLastAsClrException(); } + + return ob; } - public static void ThrowIfIsNotZero(int value) + internal static void ThrowIfIsNotZero(int value) { if (value != 0) { - throw new PythonException(); + throw ThrowLastAsClrException(); } } } diff --git a/src/runtime/runtime.cs b/src/runtime/runtime.cs index 3d2610fea..e9a020ed2 100644 --- a/src/runtime/runtime.cs +++ b/src/runtime/runtime.cs @@ -1955,10 +1955,10 @@ internal static IntPtr PyMem_Realloc(IntPtr ptr, long size) internal static extern IntPtr PyErr_Occurred(); [DllImport(_PythonDll, CallingConvention = CallingConvention.Cdecl)] - internal static extern void PyErr_Fetch(out IntPtr ob, out IntPtr val, out IntPtr tb); + internal static extern void PyErr_Fetch(out NewReference type, out NewReference value, out NewReference traceback); [DllImport(_PythonDll, CallingConvention = CallingConvention.Cdecl)] - internal static extern void PyErr_Restore(IntPtr ob, IntPtr val, IntPtr tb); + internal static extern void PyErr_Restore(NewReference type, NewReference value, NewReference traceback); [DllImport(_PythonDll, CallingConvention = CallingConvention.Cdecl)] internal static extern void PyErr_Clear(); From d45c39adf05db60982284bcade992ac7f3e374d8 Mon Sep 17 00:00:00 2001 From: Victor Milovanov Date: Mon, 24 Feb 2020 17:22:28 -0800 Subject: [PATCH 02/39] allows codecs to encode and decode thrown exceptions --- src/embed_tests/Codecs.cs | 53 +++++++++++++++++++ .../Python.EmbeddingTest.15.csproj | 2 +- src/runtime/exceptions.cs | 2 +- 3 files changed, 55 insertions(+), 2 deletions(-) diff --git a/src/embed_tests/Codecs.cs b/src/embed_tests/Codecs.cs index 18fcd32d1..0cd723338 100644 --- a/src/embed_tests/Codecs.cs +++ b/src/embed_tests/Codecs.cs @@ -82,6 +82,59 @@ static void TupleRoundtripGeneric() { Assert.AreEqual(expected: tuple, actual: restored); } } + + const string TestExceptionMessage = "Hello World!"; + [Test] + public void ExceptionEncoded() { + PyObjectConversions.RegisterEncoder(new ValueErrorCodec()); + void CallMe() => throw new ValueErrorWrapper(TestExceptionMessage); + var callMeAction = new Action(CallMe); + using var _ = Py.GIL(); + using var scope = Py.CreateScope(); + scope.Exec(@" +def call(func): + try: + func() + except ValueError as e: + return str(e) +"); + var callFunc = scope.Get("call"); + string message = callFunc.Invoke(callMeAction.ToPython()).As(); + Assert.AreEqual(TestExceptionMessage, message); + } + + [Test] + public void ExceptionDecoded() { + PyObjectConversions.RegisterDecoder(new ValueErrorCodec()); + using var _ = Py.GIL(); + using var scope = Py.CreateScope(); + var error = Assert.Throws(() => PythonEngine.Exec( + $"raise ValueError('{TestExceptionMessage}')")); + Assert.AreEqual(TestExceptionMessage, error.Message); + } + + class ValueErrorWrapper : Exception { + public ValueErrorWrapper(string message) : base(message) { } + } + + class ValueErrorCodec : IPyObjectEncoder, IPyObjectDecoder { + public bool CanDecode(PyObject objectType, Type targetType) + => this.CanEncode(targetType) && objectType.Equals(PythonEngine.Eval("ValueError")); + + public bool CanEncode(Type type) => type == typeof(ValueErrorWrapper) + || typeof(ValueErrorWrapper).IsSubclassOf(type); + + public bool TryDecode(PyObject pyObj, out T value) { + var message = pyObj.GetAttr("args")[0].As(); + value = (T)(object)new ValueErrorWrapper(message); + return true; + } + + public PyObject TryEncode(object value) { + var error = (ValueErrorWrapper)value; + return PythonEngine.Eval("ValueError").Invoke(error.Message.ToPython()); + } + } } /// diff --git a/src/embed_tests/Python.EmbeddingTest.15.csproj b/src/embed_tests/Python.EmbeddingTest.15.csproj index e163c9242..9757c9294 100644 --- a/src/embed_tests/Python.EmbeddingTest.15.csproj +++ b/src/embed_tests/Python.EmbeddingTest.15.csproj @@ -23,7 +23,7 @@ ..\..\ $(SolutionDir)\bin\ $(OutputPath)\$(TargetFramework)_publish - 7.3 + 8.0 prompt $(PYTHONNET_DEFINE_CONSTANTS) XPLAT diff --git a/src/runtime/exceptions.cs b/src/runtime/exceptions.cs index c7837f7b9..336362685 100644 --- a/src/runtime/exceptions.cs +++ b/src/runtime/exceptions.cs @@ -283,7 +283,7 @@ public static void SetError(Exception e) return; } - IntPtr op = CLRObject.GetInstHandle(e); + IntPtr op = Converter.ToPython(e); IntPtr etype = Runtime.PyObject_GetAttrString(op, "__class__"); Runtime.PyErr_SetObject(etype, op); Runtime.XDecref(etype); From 0d941c576bc676c718bac189d228aa261018c307 Mon Sep 17 00:00:00 2001 From: Victor Milovanov Date: Mon, 4 May 2020 18:27:01 -0700 Subject: [PATCH 03/39] turned on CLR exception marshaling --- src/runtime/delegatemanager.cs | 3 +-- src/runtime/exceptions.cs | 4 ++-- src/runtime/pydict.cs | 12 ++++++------ src/runtime/pyiter.cs | 2 +- src/runtime/pylist.cs | 14 ++++++------- src/runtime/pyobject.cs | 36 +++++++++++++++++----------------- src/runtime/pyscope.cs | 16 +++++++-------- src/runtime/pysequence.cs | 12 ++++++------ src/runtime/pythonengine.cs | 2 +- src/runtime/runtime.cs | 2 +- 10 files changed, 51 insertions(+), 52 deletions(-) diff --git a/src/runtime/delegatemanager.cs b/src/runtime/delegatemanager.cs index bd8f1ee4c..7dffe422b 100644 --- a/src/runtime/delegatemanager.cs +++ b/src/runtime/delegatemanager.cs @@ -257,8 +257,7 @@ public object TrueDispatch(ArrayList args) if (op == IntPtr.Zero) { - var e = new PythonException(); - throw e; + throw PythonException.ThrowLastAsClrException(); } if (rtype == typeof(void)) diff --git a/src/runtime/exceptions.cs b/src/runtime/exceptions.cs index 336362685..cb3089fe3 100644 --- a/src/runtime/exceptions.cs +++ b/src/runtime/exceptions.cs @@ -197,7 +197,7 @@ internal static void ErrorCheck(IntPtr pointer) { if (pointer == IntPtr.Zero) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } } @@ -208,7 +208,7 @@ internal static void ErrorOccurredCheck(IntPtr pointer) { if (pointer == IntPtr.Zero || ErrorOccurred()) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } } diff --git a/src/runtime/pydict.cs b/src/runtime/pydict.cs index 7ff7a83c8..116d60e39 100644 --- a/src/runtime/pydict.cs +++ b/src/runtime/pydict.cs @@ -34,7 +34,7 @@ public PyDict() obj = Runtime.PyDict_New(); if (obj == IntPtr.Zero) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } } @@ -108,7 +108,7 @@ public PyObject Keys() IntPtr items = Runtime.PyDict_Keys(obj); if (items == IntPtr.Zero) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } return new PyObject(items); } @@ -125,7 +125,7 @@ public PyObject Values() IntPtr items = Runtime.PyDict_Values(obj); if (items == IntPtr.Zero) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } return new PyObject(items); } @@ -144,7 +144,7 @@ public PyObject Items() { if (items.IsNull()) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } return items.MoveToPyObject(); @@ -167,7 +167,7 @@ public PyDict Copy() IntPtr op = Runtime.PyDict_Copy(obj); if (op == IntPtr.Zero) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } return new PyDict(op); } @@ -184,7 +184,7 @@ public void Update(PyObject other) int result = Runtime.PyDict_Update(obj, other.obj); if (result < 0) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } } diff --git a/src/runtime/pyiter.cs b/src/runtime/pyiter.cs index ee07bcecf..5b0f42bfa 100644 --- a/src/runtime/pyiter.cs +++ b/src/runtime/pyiter.cs @@ -36,7 +36,7 @@ public PyIter(PyObject iterable) obj = Runtime.PyObject_GetIter(iterable.obj); if (obj == IntPtr.Zero) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } } diff --git a/src/runtime/pylist.cs b/src/runtime/pylist.cs index 347cc3000..40e162c6a 100644 --- a/src/runtime/pylist.cs +++ b/src/runtime/pylist.cs @@ -53,7 +53,7 @@ public PyList() obj = Runtime.PyList_New(0); if (obj == IntPtr.Zero) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } } @@ -75,7 +75,7 @@ public PyList(PyObject[] items) int r = Runtime.PyList_SetItem(obj, i, ptr); if (r < 0) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } } } @@ -106,7 +106,7 @@ public static PyList AsList(PyObject value) IntPtr op = Runtime.PySequence_List(value.obj); if (op == IntPtr.Zero) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } return new PyList(op); } @@ -123,7 +123,7 @@ public void Append(PyObject item) int r = Runtime.PyList_Append(this.Reference, item.obj); if (r < 0) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } } @@ -138,7 +138,7 @@ public void Insert(int index, PyObject item) int r = Runtime.PyList_Insert(this.Reference, index, item.obj); if (r < 0) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } } @@ -154,7 +154,7 @@ public void Reverse() int r = Runtime.PyList_Reverse(this.Reference); if (r < 0) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } } @@ -170,7 +170,7 @@ public void Sort() int r = Runtime.PyList_Sort(this.Reference); if (r < 0) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } } } diff --git a/src/runtime/pyobject.cs b/src/runtime/pyobject.cs index 74e31f7e5..3b5edf73e 100644 --- a/src/runtime/pyobject.cs +++ b/src/runtime/pyobject.cs @@ -311,7 +311,7 @@ public PyObject GetAttr(string name) IntPtr op = Runtime.PyObject_GetAttrString(obj, name); if (op == IntPtr.Zero) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } return new PyObject(op); } @@ -353,7 +353,7 @@ public PyObject GetAttr(PyObject name) IntPtr op = Runtime.PyObject_GetAttr(obj, name.obj); if (op == IntPtr.Zero) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } return new PyObject(op); } @@ -396,7 +396,7 @@ public void SetAttr(string name, PyObject value) int r = Runtime.PyObject_SetAttrString(obj, name, value.obj); if (r < 0) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } } @@ -417,7 +417,7 @@ public void SetAttr(PyObject name, PyObject value) int r = Runtime.PyObject_SetAttr(obj, name.obj, value.obj); if (r < 0) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } } @@ -436,7 +436,7 @@ public void DelAttr(string name) int r = Runtime.PyObject_SetAttrString(obj, name, IntPtr.Zero); if (r < 0) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } } @@ -456,7 +456,7 @@ public void DelAttr(PyObject name) int r = Runtime.PyObject_SetAttr(obj, name.obj, IntPtr.Zero); if (r < 0) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } } @@ -476,7 +476,7 @@ public virtual PyObject GetItem(PyObject key) IntPtr op = Runtime.PyObject_GetItem(obj, key.obj); if (op == IntPtr.Zero) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } return new PyObject(op); } @@ -534,7 +534,7 @@ public virtual void SetItem(PyObject key, PyObject value) int r = Runtime.PyObject_SetItem(obj, key.obj, value.obj); if (r < 0) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } } @@ -593,7 +593,7 @@ public virtual void DelItem(PyObject key) int r = Runtime.PyObject_DelItem(obj, key.obj); if (r < 0) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } } @@ -708,7 +708,7 @@ public PyObject GetIterator() IntPtr r = Runtime.PyObject_GetIter(obj); if (r == IntPtr.Zero) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } return new PyObject(r); } @@ -744,7 +744,7 @@ public PyObject Invoke(params PyObject[] args) t.Dispose(); if (r == IntPtr.Zero) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } return new PyObject(r); } @@ -764,7 +764,7 @@ public PyObject Invoke(PyTuple args) IntPtr r = Runtime.PyObject_Call(obj, args.obj, IntPtr.Zero); if (r == IntPtr.Zero) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } return new PyObject(r); } @@ -787,7 +787,7 @@ public PyObject Invoke(PyObject[] args, PyDict kw) t.Dispose(); if (r == IntPtr.Zero) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } return new PyObject(r); } @@ -807,7 +807,7 @@ public PyObject Invoke(PyTuple args, PyDict kw) IntPtr r = Runtime.PyObject_Call(obj, args.obj, kw?.obj ?? IntPtr.Zero); if (r == IntPtr.Zero) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } return new PyObject(r); } @@ -1028,7 +1028,7 @@ public PyList Dir() IntPtr r = Runtime.PyObject_Dir(obj); if (r == IntPtr.Zero) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } return new PyList(r); } @@ -1086,7 +1086,7 @@ public override bool Equals(object o) int r = Runtime.PyObject_Compare(obj, ((PyObject)o).obj); if (Exceptions.ErrorOccurred()) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } return r == 0; } @@ -1127,7 +1127,7 @@ public override bool TrySetMember(SetMemberBinder binder, object value) int r = Runtime.PyObject_SetAttrString(obj, binder.Name, ptr); if (r < 0) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } Runtime.XDecref(ptr); return true; @@ -1199,7 +1199,7 @@ private static void AddArgument(IntPtr argtuple, int i, object target) if (Runtime.PyTuple_SetItem(argtuple, i, ptr) < 0) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } } diff --git a/src/runtime/pyscope.cs b/src/runtime/pyscope.cs index 8738824f5..b22f7d213 100644 --- a/src/runtime/pyscope.cs +++ b/src/runtime/pyscope.cs @@ -186,7 +186,7 @@ public void ImportAll(PyScope scope) int result = Runtime.PyDict_Update(variables, scope.variables); if (result < 0) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } } @@ -206,7 +206,7 @@ public void ImportAll(PyObject module) int result = Runtime.PyDict_Update(variables, module_dict); if (result < 0) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } } @@ -221,7 +221,7 @@ public void ImportAll(PyDict dict) int result = Runtime.PyDict_Update(variables, dict.obj); if (result < 0) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } } @@ -333,7 +333,7 @@ private void Exec(string code, IntPtr _globals, IntPtr _locals) Runtime.CheckExceptionOccurred(); if (!reference.IsNone()) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } } finally @@ -364,7 +364,7 @@ private void Set(string name, IntPtr value) int r = Runtime.PyObject_SetItem(variables, pyKey.obj, value); if (r < 0) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } } } @@ -383,7 +383,7 @@ public void Remove(string name) int r = Runtime.PyObject_DelItem(variables, pyKey.obj); if (r < 0) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } } } @@ -438,7 +438,7 @@ public bool TryGet(string name, out PyObject value) IntPtr op = Runtime.PyObject_GetItem(variables, pyKey.obj); if (op == IntPtr.Zero) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } if (op == Runtime.PyNone) { @@ -577,7 +577,7 @@ internal PyScope NewScope(string name) var module = Runtime.PyModule_New(name); if (module == IntPtr.Zero) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } return new PyScope(module, this); } diff --git a/src/runtime/pysequence.cs b/src/runtime/pysequence.cs index bfaee79a6..b1e89b835 100644 --- a/src/runtime/pysequence.cs +++ b/src/runtime/pysequence.cs @@ -44,7 +44,7 @@ public PyObject GetSlice(int i1, int i2) IntPtr op = Runtime.PySequence_GetSlice(obj, i1, i2); if (op == IntPtr.Zero) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } return new PyObject(op); } @@ -61,7 +61,7 @@ public void SetSlice(int i1, int i2, PyObject v) int r = Runtime.PySequence_SetSlice(obj, i1, i2, v.obj); if (r < 0) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } } @@ -77,7 +77,7 @@ public void DelSlice(int i1, int i2) int r = Runtime.PySequence_DelSlice(obj, i1, i2); if (r < 0) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } } @@ -113,7 +113,7 @@ public bool Contains(PyObject item) int r = Runtime.PySequence_Contains(obj, item.obj); if (r < 0) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } return r != 0; } @@ -131,7 +131,7 @@ public PyObject Concat(PyObject other) IntPtr op = Runtime.PySequence_Concat(obj, other.obj); if (op == IntPtr.Zero) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } return new PyObject(op); } @@ -149,7 +149,7 @@ public PyObject Repeat(int count) IntPtr op = Runtime.PySequence_Repeat(obj, count); if (op == IntPtr.Zero) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } return new PyObject(op); } diff --git a/src/runtime/pythonengine.cs b/src/runtime/pythonengine.cs index 18c4698b4..6e0716166 100644 --- a/src/runtime/pythonengine.cs +++ b/src/runtime/pythonengine.cs @@ -548,7 +548,7 @@ public static void Exec(string code, IntPtr? globals = null, IntPtr? locals = nu { if (result.obj != Runtime.PyNone) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } } } diff --git a/src/runtime/runtime.cs b/src/runtime/runtime.cs index e9a020ed2..84c315245 100644 --- a/src/runtime/runtime.cs +++ b/src/runtime/runtime.cs @@ -494,7 +494,7 @@ internal static void CheckExceptionOccurred() { if (PyErr_Occurred() != IntPtr.Zero) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } } From 02cd234a734ea550565aff953602f4f1978ae565 Mon Sep 17 00:00:00 2001 From: Victor Milovanov Date: Mon, 4 May 2020 20:17:37 -0700 Subject: [PATCH 04/39] revert to C# 7.3 --- src/embed_tests/Codecs.cs | 26 +++++++++++-------- .../Python.EmbeddingTest.15.csproj | 2 +- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/src/embed_tests/Codecs.cs b/src/embed_tests/Codecs.cs index 0cd723338..038510951 100644 --- a/src/embed_tests/Codecs.cs +++ b/src/embed_tests/Codecs.cs @@ -89,28 +89,32 @@ public void ExceptionEncoded() { PyObjectConversions.RegisterEncoder(new ValueErrorCodec()); void CallMe() => throw new ValueErrorWrapper(TestExceptionMessage); var callMeAction = new Action(CallMe); - using var _ = Py.GIL(); - using var scope = Py.CreateScope(); - scope.Exec(@" + using (var _ = Py.GIL()) + using (var scope = Py.CreateScope()) + { + scope.Exec(@" def call(func): try: func() except ValueError as e: return str(e) "); - var callFunc = scope.Get("call"); - string message = callFunc.Invoke(callMeAction.ToPython()).As(); - Assert.AreEqual(TestExceptionMessage, message); + var callFunc = scope.Get("call"); + string message = callFunc.Invoke(callMeAction.ToPython()).As(); + Assert.AreEqual(TestExceptionMessage, message); + } } [Test] public void ExceptionDecoded() { PyObjectConversions.RegisterDecoder(new ValueErrorCodec()); - using var _ = Py.GIL(); - using var scope = Py.CreateScope(); - var error = Assert.Throws(() => PythonEngine.Exec( - $"raise ValueError('{TestExceptionMessage}')")); - Assert.AreEqual(TestExceptionMessage, error.Message); + using (var _ = Py.GIL()) + using (var scope = Py.CreateScope()) + { + var error = Assert.Throws(() + => PythonEngine.Exec($"raise ValueError('{TestExceptionMessage}')")); + Assert.AreEqual(TestExceptionMessage, error.Message); + } } class ValueErrorWrapper : Exception { diff --git a/src/embed_tests/Python.EmbeddingTest.15.csproj b/src/embed_tests/Python.EmbeddingTest.15.csproj index 9757c9294..e163c9242 100644 --- a/src/embed_tests/Python.EmbeddingTest.15.csproj +++ b/src/embed_tests/Python.EmbeddingTest.15.csproj @@ -23,7 +23,7 @@ ..\..\ $(SolutionDir)\bin\ $(OutputPath)\$(TargetFramework)_publish - 8.0 + 7.3 prompt $(PYTHONNET_DEFINE_CONSTANTS) XPLAT From defb200955d0789aa68c82ce6fb3714408ca660e Mon Sep 17 00:00:00 2001 From: Victor Milovanov Date: Mon, 4 May 2020 22:07:57 -0700 Subject: [PATCH 05/39] fixed Restore when not all exception fields are set --- src/runtime/pyobject.cs | 5 +++-- src/runtime/pythonexception.cs | 5 ++++- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/runtime/pyobject.cs b/src/runtime/pyobject.cs index 3b5edf73e..6986256a4 100644 --- a/src/runtime/pyobject.cs +++ b/src/runtime/pyobject.cs @@ -33,8 +33,9 @@ public class PyObject : DynamicObject, IEnumerable, IPyDisposable public static PyObject None => new PyObject(new BorrowedReference(Runtime.PyNone)); internal BorrowedReference Reference => new BorrowedReference(this.obj); - internal NewReference MakeNewReference() - => NewReference.DangerousFromPointer(Runtime.SelfIncRef(this.obj)); + internal NewReference MakeNewReferenceOrNull() + => NewReference.DangerousFromPointer( + this.obj == IntPtr.Zero ? IntPtr.Zero : Runtime.SelfIncRef(this.obj)); /// /// PyObject Constructor diff --git a/src/runtime/pythonexception.cs b/src/runtime/pythonexception.cs index aa4f4df9b..d0523927a 100644 --- a/src/runtime/pythonexception.cs +++ b/src/runtime/pythonexception.cs @@ -223,7 +223,10 @@ public void Restore() if (this.disposed) throw new ObjectDisposedException(nameof(PythonException)); IntPtr gs = PythonEngine.AcquireLock(); - Runtime.PyErr_Restore(_type.MakeNewReference(), _value.MakeNewReference(), _pyTB.MakeNewReference()); + Runtime.PyErr_Restore( + _type.MakeNewReferenceOrNull(), + _value.MakeNewReferenceOrNull(), + _pyTB.MakeNewReferenceOrNull()); PythonEngine.ReleaseLock(gs); } From 945dc85989ddd76a40245a53b3dbcd47cc3b2f4b Mon Sep 17 00:00:00 2001 From: Victor Milovanov Date: Mon, 4 May 2020 22:08:18 -0700 Subject: [PATCH 06/39] cleaned PythonException code up a bit --- src/runtime/pythonexception.cs | 67 +++++++++++++++------------------- 1 file changed, 29 insertions(+), 38 deletions(-) diff --git a/src/runtime/pythonexception.cs b/src/runtime/pythonexception.cs index d0523927a..914f421ef 100644 --- a/src/runtime/pythonexception.cs +++ b/src/runtime/pythonexception.cs @@ -29,14 +29,12 @@ public PythonException() _pyTB = traceback.MoveToPyObjectOrNull(); if (_type != null && _value != null) { - string message; using (PyObject pyTypeName = _type.GetAttr("__name__")) { _pythonTypeName = pyTypeName.ToString(); } - message = _value.ToString(); - _message = _pythonTypeName + " : " + message; + _message = _pythonTypeName + " : " + _value; } if (_pyTB != null) { @@ -45,19 +43,17 @@ public PythonException() PythonEngine.ReleaseLock(gs); } - private PythonException(BorrowedReference pyTypeHandle, - BorrowedReference pyValueHandle, - BorrowedReference pyTracebackHandle, - string message, string pythonTypeName, string traceback, + private PythonException(PyObject type, PyObject value, PyObject traceback, + string message, string pythonTypeName, string tracebackText, Exception innerException) : base(message, innerException) { - _type = PyObject.FromNullableReference(pyTypeHandle); - _value = PyObject.FromNullableReference(pyValueHandle); - _pyTB = PyObject.FromNullableReference(pyTracebackHandle); + _type = type; + _value = value; + _pyTB = traceback; _message = message; _pythonTypeName = pythonTypeName ?? _pythonTypeName; - _traceback = traceback ?? _traceback; + _traceback = tracebackText ?? _traceback; } internal static Exception FromPyErr() @@ -143,7 +139,7 @@ internal static Exception ThrowLastAsClrException() static Exception FromPyErr(BorrowedReference typeHandle, BorrowedReference valueHandle, BorrowedReference tracebackHandle) { Exception inner = null; - string pythonTypeName = null, msg = "", traceback = null; + string pythonTypeName = null, msg = "", tracebackText = null; var clrObject = ManagedType.GetManagedObject(valueHandle) as CLRObject; if (clrObject?.inst is Exception e) @@ -151,7 +147,11 @@ static Exception FromPyErr(BorrowedReference typeHandle, BorrowedReference value return e; } - if (!typeHandle.IsNull && !valueHandle.IsNull) + var type = PyObject.FromNullableReference(typeHandle); + var value = PyObject.FromNullableReference(valueHandle); + var traceback = PyObject.FromNullableReference(tracebackHandle); + + if (type != null && value != null) { if (PyObjectConversions.TryDecode(valueHandle, typeHandle, typeof(Exception), out object decoded) && decoded is Exception decodedException) @@ -159,42 +159,33 @@ static Exception FromPyErr(BorrowedReference typeHandle, BorrowedReference value return decodedException; } - string type; - string message; - using (var pyType = new PyObject(typeHandle)) - using (PyObject pyTypeName = pyType.GetAttr("__name__")) + using (PyObject pyTypeName = type.GetAttr("__name__")) { - type = pyTypeName.ToString(); + pythonTypeName = pyTypeName.ToString(); } - pythonTypeName = type; - - using (var pyValue = new PyObject(valueHandle)) + var cause = value.GetAttr("__cause__", null); + if (cause != null && cause.Handle != Runtime.PyNone) { - message = pyValue.ToString(); - var cause = pyValue.GetAttr("__cause__", null); - if (cause != null && cause.Handle != Runtime.PyNone) + using (var innerTraceback = cause.GetAttr("__traceback__", null)) { - using (var innerTraceback = cause.GetAttr("__traceback__", null)) - { - inner = FromPyErr( - typeHandle: cause.GetPythonTypeReference(), - valueHandle: cause.Reference, - tracebackHandle: innerTraceback is null - ? BorrowedReference.Null - : innerTraceback.Reference); - } + inner = FromPyErr( + typeHandle: cause.GetPythonTypeReference(), + valueHandle: cause.Reference, + tracebackHandle: innerTraceback is null + ? BorrowedReference.Null + : innerTraceback.Reference); } } - msg = type + " : " + message; + msg = pythonTypeName + " : " + value; } - if (!tracebackHandle.IsNull) + if (traceback != null) { - traceback = TracebackToString(new PyObject(tracebackHandle)); + tracebackText = TracebackToString(traceback); } - return new PythonException(typeHandle, valueHandle, tracebackHandle, - msg, pythonTypeName, traceback, inner); + return new PythonException(type, value, traceback, + msg, pythonTypeName, tracebackText, inner); } static string TracebackToString(PyObject traceback) From 3d95e60fa9282d746355e0223a919a57f5a32279 Mon Sep 17 00:00:00 2001 From: Victor Milovanov Date: Tue, 5 May 2020 00:05:29 -0700 Subject: [PATCH 07/39] remove unused overloads of FromPyErr --- src/runtime/pythonexception.cs | 39 ---------------------------------- 1 file changed, 39 deletions(-) diff --git a/src/runtime/pythonexception.cs b/src/runtime/pythonexception.cs index 914f421ef..dd308f3c5 100644 --- a/src/runtime/pythonexception.cs +++ b/src/runtime/pythonexception.cs @@ -56,45 +56,6 @@ private PythonException(PyObject type, PyObject value, PyObject traceback, _traceback = tracebackText ?? _traceback; } - internal static Exception FromPyErr() - { - Runtime.PyErr_Fetch(out var type, out var value, out var traceback); - try - { - return FromPyErr( - typeHandle: type, - valueHandle: value, - tracebackHandle: traceback); - } - finally - { - type.Dispose(); - value.Dispose(); - traceback.Dispose(); - } - } - - internal static Exception FromPyErrOrNull() - { - Runtime.PyErr_Fetch(out var type, out var value, out var traceback); - try - { - if (value.IsNull() && type.IsNull() && traceback.IsNull()) - { - return null; - } - - var result = FromPyErr(type, value, traceback); - return result; - } - finally - { - type.Dispose(); - value.Dispose(); - traceback.Dispose(); - } - } - /// /// Rethrows the last Python exception as corresponding CLR exception. /// It is recommended to call this as throw ThrowLastAsClrException() From d6679987886b82e30f01698a32165bdb54269753 Mon Sep 17 00:00:00 2001 From: Victor Milovanov Date: Tue, 5 May 2020 00:06:51 -0700 Subject: [PATCH 08/39] capture exception on Exceptions.SetError, restore in ThrowLastAsClrException --- src/runtime/exceptions.cs | 7 +++++++ src/runtime/pythonexception.cs | 29 ++++++++++++++++++++--------- 2 files changed, 27 insertions(+), 9 deletions(-) diff --git a/src/runtime/exceptions.cs b/src/runtime/exceptions.cs index cb3089fe3..d450c8137 100644 --- a/src/runtime/exceptions.cs +++ b/src/runtime/exceptions.cs @@ -1,5 +1,6 @@ using System; using System.Reflection; +using System.Runtime.ExceptionServices; using System.Runtime.InteropServices; namespace Python.Runtime @@ -285,6 +286,12 @@ public static void SetError(Exception e) IntPtr op = Converter.ToPython(e); IntPtr etype = Runtime.PyObject_GetAttrString(op, "__class__"); +#if NETSTANDARD + var exceptionInfo = ExceptionDispatchInfo.Capture(e); + Runtime.XDecref(op); + op = Converter.ToPython(exceptionInfo); +#endif + Runtime.PyErr_SetObject(etype, op); Runtime.XDecref(etype); Runtime.XDecref(op); diff --git a/src/runtime/pythonexception.cs b/src/runtime/pythonexception.cs index dd308f3c5..f917bbaf6 100644 --- a/src/runtime/pythonexception.cs +++ b/src/runtime/pythonexception.cs @@ -66,26 +66,30 @@ internal static Exception ThrowLastAsClrException() IntPtr gs = PythonEngine.AcquireLock(); try { - Runtime.PyErr_Fetch(out var pyTypeHandle, out var pyValueHandle, out var pyTracebackHandle); + Runtime.PyErr_Fetch(out var type, out var value, out var traceback); try { - var clrObject = ManagedType.GetManagedObject(pyValueHandle) as CLRObject; - if (clrObject?.inst is Exception e) - { + var clrObject = ManagedType.GetManagedObject(value) as CLRObject; #if NETSTANDARD - ExceptionDispatchInfo.Capture(e).Throw(); + if (clrObject?.inst is ExceptionDispatchInfo storedException) + { + storedException.Throw(); + throw storedException.SourceException; // unreachable + } #endif + if (clrObject?.inst is Exception e) + { throw e; } - var result = FromPyErr(pyTypeHandle, pyValueHandle, pyTracebackHandle); + var result = FromPyErr(type, value, traceback); throw result; } finally { - pyTypeHandle.Dispose(); - pyValueHandle.Dispose(); - pyTracebackHandle.Dispose(); + type.Dispose(); + value.Dispose(); + traceback.Dispose(); } } finally @@ -108,6 +112,13 @@ static Exception FromPyErr(BorrowedReference typeHandle, BorrowedReference value return e; } +#if NETSTANDARD + if (clrObject?.inst is ExceptionDispatchInfo exceptionDispatchInfo) + { + return exceptionDispatchInfo.SourceException; + } +#endif + var type = PyObject.FromNullableReference(typeHandle); var value = PyObject.FromNullableReference(valueHandle); var traceback = PyObject.FromNullableReference(tracebackHandle); From bec8b7dfb83a09c96b3083b7188105101cbce46b Mon Sep 17 00:00:00 2001 From: Victor Milovanov Date: Tue, 5 May 2020 00:39:01 -0700 Subject: [PATCH 09/39] fixed failure in ExceptionEncoded test case caused by ExceptionDispatchInfo masking exception object --- src/runtime/converter.cs | 2 ++ src/runtime/exceptions.cs | 6 ++-- src/runtime/pythonexception.cs | 59 +++++++++++++++++++++++++++------- src/runtime/runtime.cs | 3 ++ 4 files changed, 57 insertions(+), 13 deletions(-) diff --git a/src/runtime/converter.cs b/src/runtime/converter.cs index 3add8aba0..d4c407214 100644 --- a/src/runtime/converter.cs +++ b/src/runtime/converter.cs @@ -293,6 +293,8 @@ internal static bool ToManaged(IntPtr value, Type type, } + internal static bool ToManagedValue(BorrowedReference value, Type obType, out object result, bool setError) + => ToManagedValue(value.DangerousGetAddress(), obType, out result, setError); internal static bool ToManagedValue(IntPtr value, Type obType, out object result, bool setError) { diff --git a/src/runtime/exceptions.cs b/src/runtime/exceptions.cs index d450c8137..3d8d66b29 100644 --- a/src/runtime/exceptions.cs +++ b/src/runtime/exceptions.cs @@ -262,6 +262,7 @@ public static void SetError(IntPtr ob, IntPtr value) Runtime.PyErr_SetObject(ob, value); } + internal const string DispatchInfoAttribute = "__dispatch_info__"; /// /// SetError Method /// @@ -288,8 +289,9 @@ public static void SetError(Exception e) IntPtr etype = Runtime.PyObject_GetAttrString(op, "__class__"); #if NETSTANDARD var exceptionInfo = ExceptionDispatchInfo.Capture(e); - Runtime.XDecref(op); - op = Converter.ToPython(exceptionInfo); + IntPtr pyInfo = Converter.ToPython(exceptionInfo); + Runtime.PyObject_SetAttrString(op, DispatchInfoAttribute, pyInfo); + Runtime.XDecref(pyInfo); #endif Runtime.PyErr_SetObject(etype, op); diff --git a/src/runtime/pythonexception.cs b/src/runtime/pythonexception.cs index f917bbaf6..7f28fdb51 100644 --- a/src/runtime/pythonexception.cs +++ b/src/runtime/pythonexception.cs @@ -69,14 +69,19 @@ internal static Exception ThrowLastAsClrException() Runtime.PyErr_Fetch(out var type, out var value, out var traceback); try { - var clrObject = ManagedType.GetManagedObject(value) as CLRObject; #if NETSTANDARD - if (clrObject?.inst is ExceptionDispatchInfo storedException) + if (!value.IsNull()) { - storedException.Throw(); - throw storedException.SourceException; // unreachable + var exceptionInfo = TryGetDispatchInfo(value); + if (exceptionInfo != null) + { + exceptionInfo.Throw(); + throw exceptionInfo.SourceException; // unreachable + } } #endif + + var clrObject = ManagedType.GetManagedObject(value) as CLRObject; if (clrObject?.inst is Exception e) { throw e; @@ -98,6 +103,37 @@ internal static Exception ThrowLastAsClrException() } } +#if NETSTANDARD + static ExceptionDispatchInfo TryGetDispatchInfo(BorrowedReference exception) + { + if (exception.IsNull) return null; + + var pyInfo = Runtime.PyObject_GetAttrString(exception, Exceptions.DispatchInfoAttribute); + if (pyInfo.IsNull()) + { + if (Exceptions.ExceptionMatches(Exceptions.AttributeError)) + { + Exceptions.Clear(); + } + return null; + } + + try + { + if (Converter.ToManagedValue(pyInfo, typeof(ExceptionDispatchInfo), out object result, setError: false)) + { + return (ExceptionDispatchInfo)result; + } + + return null; + } + finally + { + pyInfo.Dispose(); + } + } +#endif + /// /// Requires lock to be acquired elsewhere /// @@ -106,19 +142,20 @@ static Exception FromPyErr(BorrowedReference typeHandle, BorrowedReference value Exception inner = null; string pythonTypeName = null, msg = "", tracebackText = null; - var clrObject = ManagedType.GetManagedObject(valueHandle) as CLRObject; - if (clrObject?.inst is Exception e) - { - return e; - } - #if NETSTANDARD - if (clrObject?.inst is ExceptionDispatchInfo exceptionDispatchInfo) + var exceptionDispatchInfo = TryGetDispatchInfo(valueHandle); + if (exceptionDispatchInfo != null) { return exceptionDispatchInfo.SourceException; } #endif + var clrObject = ManagedType.GetManagedObject(valueHandle) as CLRObject; + if (clrObject?.inst is Exception e) + { + return e; + } + var type = PyObject.FromNullableReference(typeHandle); var value = PyObject.FromNullableReference(valueHandle); var traceback = PyObject.FromNullableReference(tracebackHandle); diff --git a/src/runtime/runtime.cs b/src/runtime/runtime.cs index 84c315245..d7f090cb2 100644 --- a/src/runtime/runtime.cs +++ b/src/runtime/runtime.cs @@ -937,7 +937,10 @@ internal static bool PyObject_IsIterable(IntPtr pointer) internal static extern int PyObject_HasAttrString(IntPtr pointer, string name); [DllImport(_PythonDll, CallingConvention = CallingConvention.Cdecl)] + [Obsolete("Use overload accepting BorrowedReference")] internal static extern IntPtr PyObject_GetAttrString(IntPtr pointer, string name); + [DllImport(_PythonDll, CallingConvention = CallingConvention.Cdecl)] + internal static extern NewReference PyObject_GetAttrString(BorrowedReference pointer, string name); [DllImport(_PythonDll, CallingConvention = CallingConvention.Cdecl)] internal static extern int PyObject_SetAttrString(IntPtr pointer, string name, IntPtr value); From 0961c94a573586a2eb1a103caf87579712c7ad2d Mon Sep 17 00:00:00 2001 From: Victor Milovanov Date: Wed, 6 May 2020 14:16:04 -0700 Subject: [PATCH 10/39] added tests for __cause__/InnerException #893 #1098 --- src/embed_tests/TestPythonException.cs | 30 ++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/src/embed_tests/TestPythonException.cs b/src/embed_tests/TestPythonException.cs index 000c32ca3..65cc18004 100644 --- a/src/embed_tests/TestPythonException.cs +++ b/src/embed_tests/TestPythonException.cs @@ -55,6 +55,36 @@ public void TestPythonErrorTypeName() } } + [Test] + public void TestNestedExceptions() + { + try + { + PythonEngine.Exec(@" +try: + raise Exception('inner') +except Exception as ex: + raise Exception('outer') from ex +"); + } + catch (PythonException ex) + { + Assert.That(ex.InnerException, Is.InstanceOf()); + Assert.That(ex.InnerException.Message, Is.EqualTo("Exception : inner")); + } + } + + [Test] + public void InnerIsEmptyWithNoCause() + { + var list = new PyList(); + PyObject foo = null; + + var ex = Assert.Throws(() => foo = list[0]); + + Assert.IsNull(ex.InnerException); + } + [Test] public void TestPythonExceptionFormat() { From 2cd8627703a764655ba6e6f8b5525c9ca5f6eb09 Mon Sep 17 00:00:00 2001 From: Victor Milovanov Date: Wed, 13 May 2020 21:18:14 -0700 Subject: [PATCH 11/39] introduced StolenReference type The new type indicates parameters of C API functions, that steal references to passed objects. --- src/runtime/NewReference.cs | 10 ++++++++++ src/runtime/Python.Runtime.csproj | 1 + src/runtime/StolenReference.cs | 18 ++++++++++++++++++ src/runtime/pyobject.cs | 2 +- src/runtime/pythonexception.cs | 6 +++--- src/runtime/runtime.cs | 2 +- 6 files changed, 34 insertions(+), 5 deletions(-) create mode 100644 src/runtime/StolenReference.cs diff --git a/src/runtime/NewReference.cs b/src/runtime/NewReference.cs index 2e27f2b2b..7abf0b708 100644 --- a/src/runtime/NewReference.cs +++ b/src/runtime/NewReference.cs @@ -33,6 +33,16 @@ public PyObject MoveToPyObject() /// public PyObject MoveToPyObjectOrNull() => this.IsNull() ? null : this.MoveToPyObject(); /// + /// Call this method to move ownership of this reference to a Python C API function, + /// that steals reference passed to it. + /// + public StolenReference Steal() + { + IntPtr rawPointer = this.pointer; + this.pointer = IntPtr.Zero; + return new StolenReference(rawPointer); + } + /// /// Removes this reference to a Python object, and sets it to null. /// public void Dispose() diff --git a/src/runtime/Python.Runtime.csproj b/src/runtime/Python.Runtime.csproj index 2e47805cb..0dc46bd53 100644 --- a/src/runtime/Python.Runtime.csproj +++ b/src/runtime/Python.Runtime.csproj @@ -146,6 +146,7 @@ + diff --git a/src/runtime/StolenReference.cs b/src/runtime/StolenReference.cs new file mode 100644 index 000000000..fb789eec5 --- /dev/null +++ b/src/runtime/StolenReference.cs @@ -0,0 +1,18 @@ +namespace Python.Runtime +{ + using System; + + /// + /// Should only be used for the arguments of Python C API functions, that steal references + /// + [NonCopyable] + readonly ref struct StolenReference + { + readonly IntPtr pointer; + + internal StolenReference(IntPtr pointer) + { + this.pointer = pointer; + } + } +} diff --git a/src/runtime/pyobject.cs b/src/runtime/pyobject.cs index 6986256a4..822cb4496 100644 --- a/src/runtime/pyobject.cs +++ b/src/runtime/pyobject.cs @@ -215,7 +215,7 @@ protected virtual void Dispose(bool disposing) { // Python requires finalizers to preserve exception: // https://docs.python.org/3/extending/newtypes.html#finalization-and-de-allocation - Runtime.PyErr_Restore(errType, errVal, traceback); + Runtime.PyErr_Restore(errType.Steal(), errVal.Steal(), traceback.Steal()); } } else diff --git a/src/runtime/pythonexception.cs b/src/runtime/pythonexception.cs index 7f28fdb51..a3c1df93b 100644 --- a/src/runtime/pythonexception.cs +++ b/src/runtime/pythonexception.cs @@ -224,9 +224,9 @@ public void Restore() IntPtr gs = PythonEngine.AcquireLock(); Runtime.PyErr_Restore( - _type.MakeNewReferenceOrNull(), - _value.MakeNewReferenceOrNull(), - _pyTB.MakeNewReferenceOrNull()); + _type.MakeNewReferenceOrNull().Steal(), + _value.MakeNewReferenceOrNull().Steal(), + _pyTB.MakeNewReferenceOrNull().Steal()); PythonEngine.ReleaseLock(gs); } diff --git a/src/runtime/runtime.cs b/src/runtime/runtime.cs index d7f090cb2..0a10d0e91 100644 --- a/src/runtime/runtime.cs +++ b/src/runtime/runtime.cs @@ -1961,7 +1961,7 @@ internal static IntPtr PyMem_Realloc(IntPtr ptr, long size) internal static extern void PyErr_Fetch(out NewReference type, out NewReference value, out NewReference traceback); [DllImport(_PythonDll, CallingConvention = CallingConvention.Cdecl)] - internal static extern void PyErr_Restore(NewReference type, NewReference value, NewReference traceback); + internal static extern void PyErr_Restore(StolenReference type, StolenReference value, StolenReference traceback); [DllImport(_PythonDll, CallingConvention = CallingConvention.Cdecl)] internal static extern void PyErr_Clear(); From e4c1b9be57ae9fe670c15189648a87277e41d923 Mon Sep 17 00:00:00 2001 From: Victor Milovanov Date: Thu, 28 May 2020 18:17:19 -0700 Subject: [PATCH 12/39] prevent undebuggable StackOverflowException during exception interop --- src/runtime/pythonexception.cs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/runtime/pythonexception.cs b/src/runtime/pythonexception.cs index a3c1df93b..d544551e0 100644 --- a/src/runtime/pythonexception.cs +++ b/src/runtime/pythonexception.cs @@ -63,6 +63,10 @@ private PythonException(PyObject type, PyObject value, PyObject traceback, /// internal static Exception ThrowLastAsClrException() { + // prevent potential interop errors in this method + // from crashing process with undebuggable StackOverflowException + RuntimeHelpers.EnsureSufficientExecutionStack(); + IntPtr gs = PythonEngine.AcquireLock(); try { From 3c0b737030466d684293d4537a10356dcf3af778 Mon Sep 17 00:00:00 2001 From: Victor Milovanov Date: Mon, 22 Jun 2020 13:49:35 -0700 Subject: [PATCH 13/39] README: added summary of new exception handling features --- CHANGELOG.md | 5 +++++ pythonnet.15.sln | 1 - 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index afb2badbd..79e1bf0be 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,11 @@ This document follows the conventions laid out in [Keep a CHANGELOG][]. ### Added +- Improved exception handling: + - exceptions can now be converted with codecs + - `InnerException` and `__cause__` are propagated properly + - .NET and Python exceptions are preserved when crossing Python/.NET boundary + ### Changed ### Fixed diff --git a/pythonnet.15.sln b/pythonnet.15.sln index ce863817f..1105f1afb 100644 --- a/pythonnet.15.sln +++ b/pythonnet.15.sln @@ -39,7 +39,6 @@ Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "conda.recipe", "conda.recip ProjectSection(SolutionItems) = preProject conda.recipe\bld.bat = conda.recipe\bld.bat conda.recipe\meta.yaml = conda.recipe\meta.yaml - conda.recipe\README.md = conda.recipe\README.md EndProjectSection EndProject Global From c0fe430e5a32851a3133f88c2462d9362cfe2055 Mon Sep 17 00:00:00 2001 From: Victor Nova Date: Thu, 8 Apr 2021 23:32:25 -0700 Subject: [PATCH 14/39] reworked `PythonException`: Removed private fields, apart from ones returned by `PyErr_Fetch`. Corresponding property values are now generated on demand. Added FetchCurrent*Raw for internal consumption. `PythonException.Type` is now of type `PyType`. Use C API functions `PyException_GetCause` and `PyException_GetTraceback` instead of trying to read via attributes by name. `PythonException` instances are no longer disposable. You can still dispose `.Type`, `.Value` and `.Traceback`, but it is not recommended, as they may be shared with other instances. --- CHANGELOG.md | 2 + src/embed_tests/Codecs.cs | 44 ++-- src/embed_tests/TestCallbacks.cs | 2 +- src/embed_tests/TestPyFloat.cs | 4 +- src/embed_tests/TestPyInt.cs | 4 +- src/embed_tests/TestPyList.cs | 2 +- src/embed_tests/TestPyLong.cs | 4 +- src/embed_tests/TestPyTuple.cs | 4 +- src/embed_tests/TestPyType.cs | 1 + src/embed_tests/TestPyWith.cs | 2 +- src/embed_tests/TestPythonException.cs | 64 +++-- src/embed_tests/pyimport.cs | 3 +- src/embed_tests/pyinitialize.cs | 2 +- src/runtime/StolenReference.cs | 34 ++- src/runtime/classmanager.cs | 2 +- src/runtime/converter.cs | 3 + src/runtime/delegatemanager.cs | 10 +- src/runtime/exceptions.cs | 67 +++-- src/runtime/finalizer.cs | 2 +- src/runtime/importhook.cs | 4 +- src/runtime/methodbinder.cs | 2 +- src/runtime/moduleobject.cs | 4 +- src/runtime/pybuffer.cs | 8 +- src/runtime/pyiter.cs | 2 +- src/runtime/pyobject.cs | 28 ++- src/runtime/pythonengine.cs | 18 +- src/runtime/pythonexception.cs | 324 ++++++++++++------------- src/runtime/pytuple.cs | 2 +- src/runtime/pytype.cs | 34 +++ src/runtime/runtime.cs | 55 +++-- src/runtime/typemanager.cs | 12 +- 31 files changed, 415 insertions(+), 334 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3081e2e84..5871e7ffb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,6 +49,7 @@ One must now either use enum members (e.g. `MyEnum.Option`), or use enum constru support for .NET Core - .NET and Python exceptions are preserved when crossing Python/.NET boundary - BREAKING: custom encoders are no longer called for instances of `System.Type` +- `PythonException.Restore` no longer clears `PythonException` instance. ### Fixed @@ -74,6 +75,7 @@ One must now either use enum members (e.g. `MyEnum.Option`), or use enum constru ### Removed - implicit assembly loading (you have to explicitly `clr.AddReference` before doing import) +- messages in `PythonException` no longer start with exception type - support for .NET Framework 4.0-4.6; Mono before 5.4. Python.NET now requires .NET Standard 2.0 (see [the matrix](https://docs.microsoft.com/en-us/dotnet/standard/net-standard#net-implementation-support)) diff --git a/src/embed_tests/Codecs.cs b/src/embed_tests/Codecs.cs index c36864e69..f0c00a6d8 100644 --- a/src/embed_tests/Codecs.cs +++ b/src/embed_tests/Codecs.cs @@ -325,56 +325,58 @@ def CanEncode(self, clr_type): const string TestExceptionMessage = "Hello World!"; [Test] - public void ExceptionEncoded() { + public void ExceptionEncoded() + { PyObjectConversions.RegisterEncoder(new ValueErrorCodec()); void CallMe() => throw new ValueErrorWrapper(TestExceptionMessage); var callMeAction = new Action(CallMe); - using (var _ = Py.GIL()) - using (var scope = Py.CreateScope()) - { - scope.Exec(@" + using var _ = Py.GIL(); + using var scope = Py.CreateScope(); + scope.Exec(@" def call(func): try: func() except ValueError as e: return str(e) "); - var callFunc = scope.Get("call"); - string message = callFunc.Invoke(callMeAction.ToPython()).As(); - Assert.AreEqual(TestExceptionMessage, message); - } + var callFunc = scope.Get("call"); + string message = callFunc.Invoke(callMeAction.ToPython()).As(); + Assert.AreEqual(TestExceptionMessage, message); } [Test] - public void ExceptionDecoded() { + public void ExceptionDecoded() + { PyObjectConversions.RegisterDecoder(new ValueErrorCodec()); - using (var _ = Py.GIL()) - using (var scope = Py.CreateScope()) - { - var error = Assert.Throws(() - => PythonEngine.Exec($"raise ValueError('{TestExceptionMessage}')")); - Assert.AreEqual(TestExceptionMessage, error.Message); - } + using var _ = Py.GIL(); + using var scope = Py.CreateScope(); + var error = Assert.Throws(() + => PythonEngine.Exec($"raise ValueError('{TestExceptionMessage}')")); + Assert.AreEqual(TestExceptionMessage, error.Message); } - class ValueErrorWrapper : Exception { + class ValueErrorWrapper : Exception + { public ValueErrorWrapper(string message) : base(message) { } } - class ValueErrorCodec : IPyObjectEncoder, IPyObjectDecoder { + class ValueErrorCodec : IPyObjectEncoder, IPyObjectDecoder + { public bool CanDecode(PyObject objectType, Type targetType) => this.CanEncode(targetType) && objectType.Equals(PythonEngine.Eval("ValueError")); public bool CanEncode(Type type) => type == typeof(ValueErrorWrapper) || typeof(ValueErrorWrapper).IsSubclassOf(type); - public bool TryDecode(PyObject pyObj, out T value) { + public bool TryDecode(PyObject pyObj, out T value) + { var message = pyObj.GetAttr("args")[0].As(); value = (T)(object)new ValueErrorWrapper(message); return true; } - public PyObject TryEncode(object value) { + public PyObject TryEncode(object value) + { var error = (ValueErrorWrapper)value; return PythonEngine.Eval("ValueError").Invoke(error.Message.ToPython()); } diff --git a/src/embed_tests/TestCallbacks.cs b/src/embed_tests/TestCallbacks.cs index 454c97578..6875fde01 100644 --- a/src/embed_tests/TestCallbacks.cs +++ b/src/embed_tests/TestCallbacks.cs @@ -24,7 +24,7 @@ public void TestNoOverloadException() { using (Py.GIL()) { dynamic callWith42 = PythonEngine.Eval("lambda f: f([42])"); var error = Assert.Throws(() => callWith42(aFunctionThatCallsIntoPython.ToPython())); - Assert.AreEqual("TypeError", error.PythonTypeName); + Assert.AreEqual("TypeError", error.Type.Name); string expectedArgTypes = "()"; StringAssert.EndsWith(expectedArgTypes, error.Message); } diff --git a/src/embed_tests/TestPyFloat.cs b/src/embed_tests/TestPyFloat.cs index 94e7026c7..906c8cb0d 100644 --- a/src/embed_tests/TestPyFloat.cs +++ b/src/embed_tests/TestPyFloat.cs @@ -95,7 +95,7 @@ public void StringBadCtor() var ex = Assert.Throws(() => a = new PyFloat(i)); - StringAssert.StartsWith("ValueError : could not convert string to float", ex.Message); + StringAssert.StartsWith("could not convert string to float", ex.Message); Assert.IsNull(a); } @@ -132,7 +132,7 @@ public void AsFloatBad() PyFloat a = null; var ex = Assert.Throws(() => a = PyFloat.AsFloat(s)); - StringAssert.StartsWith("ValueError : could not convert string to float", ex.Message); + StringAssert.StartsWith("could not convert string to float", ex.Message); Assert.IsNull(a); } } diff --git a/src/embed_tests/TestPyInt.cs b/src/embed_tests/TestPyInt.cs index 005ab466d..bd6cf23a1 100644 --- a/src/embed_tests/TestPyInt.cs +++ b/src/embed_tests/TestPyInt.cs @@ -128,7 +128,7 @@ public void TestCtorBadString() var ex = Assert.Throws(() => a = new PyInt(i)); - StringAssert.StartsWith("ValueError : invalid literal for int", ex.Message); + StringAssert.StartsWith("invalid literal for int", ex.Message); Assert.IsNull(a); } @@ -161,7 +161,7 @@ public void TestAsIntBad() PyInt a = null; var ex = Assert.Throws(() => a = PyInt.AsInt(s)); - StringAssert.StartsWith("ValueError : invalid literal for int", ex.Message); + StringAssert.StartsWith("invalid literal for int", ex.Message); Assert.IsNull(a); } diff --git a/src/embed_tests/TestPyList.cs b/src/embed_tests/TestPyList.cs index e9acfbb45..eee129f2d 100644 --- a/src/embed_tests/TestPyList.cs +++ b/src/embed_tests/TestPyList.cs @@ -41,7 +41,7 @@ public void TestStringAsListType() var ex = Assert.Throws(() => t = PyList.AsList(i)); - Assert.AreEqual("TypeError : 'int' object is not iterable", ex.Message); + Assert.AreEqual("'int' object is not iterable", ex.Message); Assert.IsNull(t); } diff --git a/src/embed_tests/TestPyLong.cs b/src/embed_tests/TestPyLong.cs index 3c155f315..6d587d064 100644 --- a/src/embed_tests/TestPyLong.cs +++ b/src/embed_tests/TestPyLong.cs @@ -144,7 +144,7 @@ public void TestCtorBadString() var ex = Assert.Throws(() => a = new PyLong(i)); - StringAssert.StartsWith("ValueError : invalid literal", ex.Message); + StringAssert.StartsWith("invalid literal", ex.Message); Assert.IsNull(a); } @@ -177,7 +177,7 @@ public void TestAsLongBad() PyLong a = null; var ex = Assert.Throws(() => a = PyLong.AsLong(s)); - StringAssert.StartsWith("ValueError : invalid literal", ex.Message); + StringAssert.StartsWith("invalid literal", ex.Message); Assert.IsNull(a); } diff --git a/src/embed_tests/TestPyTuple.cs b/src/embed_tests/TestPyTuple.cs index 362251049..5d76116aa 100644 --- a/src/embed_tests/TestPyTuple.cs +++ b/src/embed_tests/TestPyTuple.cs @@ -104,7 +104,7 @@ public void TestPyTupleInvalidAppend() var ex = Assert.Throws(() => t.Concat(s)); - StringAssert.StartsWith("TypeError : can only concatenate tuple", ex.Message); + StringAssert.StartsWith("can only concatenate tuple", ex.Message); Assert.AreEqual(0, t.Length()); Assert.IsEmpty(t); } @@ -164,7 +164,7 @@ public void TestInvalidAsTuple() var ex = Assert.Throws(() => t = PyTuple.AsTuple(i)); - Assert.AreEqual("TypeError : 'int' object is not iterable", ex.Message); + Assert.AreEqual("'int' object is not iterable", ex.Message); Assert.IsNull(t); } } diff --git a/src/embed_tests/TestPyType.cs b/src/embed_tests/TestPyType.cs index 02142b782..d3937b064 100644 --- a/src/embed_tests/TestPyType.cs +++ b/src/embed_tests/TestPyType.cs @@ -39,6 +39,7 @@ public void CanCreateHeapType() using var type = new PyType(spec); Assert.AreEqual(name, type.GetAttr("__name__").As()); + Assert.AreEqual(name, type.Name); Assert.AreEqual(docStr, type.GetAttr("__doc__").As()); } } diff --git a/src/embed_tests/TestPyWith.cs b/src/embed_tests/TestPyWith.cs index dcd539504..c6228f1b9 100644 --- a/src/embed_tests/TestPyWith.cs +++ b/src/embed_tests/TestPyWith.cs @@ -51,7 +51,7 @@ def fail(self): catch (PythonException e) { TestContext.Out.WriteLine(e.Message); - Assert.IsTrue(e.Message.Contains("ZeroDivisionError")); + Assert.IsTrue(e.Type.Name == "ZeroDivisionError"); } } diff --git a/src/embed_tests/TestPythonException.cs b/src/embed_tests/TestPythonException.cs index 702b6c3b1..0763bfb34 100644 --- a/src/embed_tests/TestPythonException.cs +++ b/src/embed_tests/TestPythonException.cs @@ -30,29 +30,29 @@ public void TestMessage() var ex = Assert.Throws(() => foo = list[0]); - Assert.AreEqual("IndexError : list index out of range", ex.Message); + Assert.AreEqual("list index out of range", ex.Message); Assert.IsNull(foo); } [Test] - public void TestNoError() + public void TestType() { - var e = new PythonException(); // There is no PyErr to fetch - Assert.AreEqual("", e.Message); + var list = new PyList(); + PyObject foo = null; + + var ex = Assert.Throws(() => foo = list[0]); + + Assert.AreEqual("IndexError", ex.Type.Name); + Assert.IsNull(foo); } [Test] - public void TestPythonErrorTypeName() + public void TestNoError() { - try - { - var module = PyModule.Import("really____unknown___module"); - Assert.Fail("Unknown module should not be loaded"); - } - catch (PythonException ex) - { - Assert.That(ex.PythonTypeName, Is.EqualTo("ModuleNotFoundError").Or.EqualTo("ImportError")); - } + // There is no PyErr to fetch + Assert.Throws(() => PythonException.FetchCurrentRaw()); + var currentError = PythonException.FetchCurrentOrNullRaw(); + Assert.IsNull(currentError); } [Test] @@ -70,7 +70,7 @@ raise Exception('outer') from ex catch (PythonException ex) { Assert.That(ex.InnerException, Is.InstanceOf()); - Assert.That(ex.InnerException.Message, Is.EqualTo("Exception : inner")); + Assert.That(ex.InnerException.Message, Is.EqualTo("inner")); } } @@ -113,13 +113,6 @@ public void TestPythonExceptionFormat() } } - [Test] - public void TestPythonExceptionFormatNoError() - { - var ex = new PythonException(); - Assert.AreEqual(ex.StackTrace, ex.Format()); - } - [Test] public void TestPythonExceptionFormatNoTraceback() { @@ -162,22 +155,19 @@ def __init__(self, val): Assert.IsTrue(scope.TryGet("TestException", out PyObject type)); PyObject str = "dummy string".ToPython(); - IntPtr typePtr = type.Handle; - IntPtr strPtr = str.Handle; - IntPtr tbPtr = Runtime.Runtime.None.Handle; - Runtime.Runtime.XIncref(typePtr); - Runtime.Runtime.XIncref(strPtr); - Runtime.Runtime.XIncref(tbPtr); + var typePtr = new NewReference(type.Reference); + var strPtr = new NewReference(str.Reference); + var tbPtr = new NewReference(Runtime.Runtime.None.Reference); Runtime.Runtime.PyErr_NormalizeException(ref typePtr, ref strPtr, ref tbPtr); - using (PyObject typeObj = new PyObject(typePtr), strObj = new PyObject(strPtr), tbObj = new PyObject(tbPtr)) - { - // the type returned from PyErr_NormalizeException should not be the same type since a new - // exception was raised by initializing the exception - Assert.AreNotEqual(type.Handle, typePtr); - // the message should now be the string from the throw exception during normalization - Assert.AreEqual("invalid literal for int() with base 10: 'dummy string'", strObj.ToString()); - } + using var typeObj = typePtr.MoveToPyObject(); + using var strObj = strPtr.MoveToPyObject(); + using var tbObj = tbPtr.MoveToPyObject(); + // the type returned from PyErr_NormalizeException should not be the same type since a new + // exception was raised by initializing the exception + Assert.AreNotEqual(type.Handle, typeObj.Handle); + // the message should now be the string from the throw exception during normalization + Assert.AreEqual("invalid literal for int() with base 10: 'dummy string'", strObj.ToString()); } } @@ -185,7 +175,7 @@ def __init__(self, val): public void TestPythonException_Normalize_ThrowsWhenErrorSet() { Exceptions.SetError(Exceptions.TypeError, "Error!"); - var pythonException = new PythonException(); + var pythonException = PythonException.FetchCurrentRaw(); Exceptions.SetError(Exceptions.TypeError, "Another error"); Assert.Throws(() => pythonException.Normalize()); } diff --git a/src/embed_tests/pyimport.cs b/src/embed_tests/pyimport.cs index f1f667961..e98461cbb 100644 --- a/src/embed_tests/pyimport.cs +++ b/src/embed_tests/pyimport.cs @@ -102,8 +102,7 @@ import clr clr.AddReference('{path}') "; - var error = Assert.Throws(() => PythonEngine.Exec(code)); - Assert.AreEqual(nameof(FileLoadException), error.PythonTypeName); + Assert.Throws(() => PythonEngine.Exec(code)); } } } diff --git a/src/embed_tests/pyinitialize.cs b/src/embed_tests/pyinitialize.cs index 66a9a3f7c..1683aeca1 100644 --- a/src/embed_tests/pyinitialize.cs +++ b/src/embed_tests/pyinitialize.cs @@ -160,7 +160,7 @@ public static void TestRunExitFuncs() string msg = e.ToString(); Runtime.Runtime.Shutdown(); - if (e.IsMatches(Exceptions.ImportError)) + if (e.Is(Exceptions.ImportError)) { Assert.Ignore("no atexit module"); } diff --git a/src/runtime/StolenReference.cs b/src/runtime/StolenReference.cs index fb789eec5..1130cff06 100644 --- a/src/runtime/StolenReference.cs +++ b/src/runtime/StolenReference.cs @@ -1,18 +1,46 @@ namespace Python.Runtime { using System; + using System.Diagnostics.Contracts; /// - /// Should only be used for the arguments of Python C API functions, that steal references + /// Should only be used for the arguments of Python C API functions, that steal references, + /// and internal constructors. /// [NonCopyable] readonly ref struct StolenReference { - readonly IntPtr pointer; + internal readonly IntPtr Pointer; internal StolenReference(IntPtr pointer) { - this.pointer = pointer; + Pointer = pointer; } + + [Pure] + public static bool operator ==(in StolenReference reference, NullOnly @null) + => reference.Pointer == IntPtr.Zero; + [Pure] + public static bool operator !=(in StolenReference reference, NullOnly @null) + => reference.Pointer != IntPtr.Zero; + + [Pure] + public override bool Equals(object obj) + { + if (obj is IntPtr ptr) + return ptr == Pointer; + + return false; + } + + [Pure] + public override int GetHashCode() => Pointer.GetHashCode(); + } + + static class StolenReferenceExtensions + { + [Pure] + public static IntPtr DangerousGetAddressOrNull(this in StolenReference reference) + => reference.Pointer; } } diff --git a/src/runtime/classmanager.cs b/src/runtime/classmanager.cs index 306962f56..d118fc273 100644 --- a/src/runtime/classmanager.cs +++ b/src/runtime/classmanager.cs @@ -131,7 +131,7 @@ internal static void SaveRuntimeData(RuntimeDataStorage storage) } else if (Exceptions.ErrorOccurred()) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } } // We modified the Type object, notify it we did. diff --git a/src/runtime/converter.cs b/src/runtime/converter.cs index cd9477a62..235b71528 100644 --- a/src/runtime/converter.cs +++ b/src/runtime/converter.cs @@ -110,6 +110,9 @@ internal static IntPtr ToPython(T value) return ToPython(value, typeof(T)); } + internal static NewReference ToPythonReference(T value) + => NewReference.DangerousFromPointer(ToPython(value, typeof(T))); + private static readonly Func IsTransparentProxy = GetIsTransparentProxy(); private static bool Never(object _) => false; diff --git a/src/runtime/delegatemanager.cs b/src/runtime/delegatemanager.cs index 440f0eef3..22f603400 100644 --- a/src/runtime/delegatemanager.cs +++ b/src/runtime/delegatemanager.cs @@ -323,7 +323,7 @@ private object TrueDispatch(object[] args) if (!Converter.ToManaged(op, t, out object newArg, true)) { Exceptions.RaiseTypeError($"The Python function did not return {t.GetElementType()} (the out parameter type)"); - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } args[i] = newArg; break; @@ -343,7 +343,7 @@ private object TrueDispatch(object[] args) if (!Converter.ToManaged(item, t, out object newArg, true)) { Exceptions.RaiseTypeError($"The Python function returned a tuple where element {i} was not {t.GetElementType()} (the out parameter type)"); - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } args[i] = newArg; } @@ -356,7 +356,7 @@ private object TrueDispatch(object[] args) if (!Converter.ToManaged(item0, rtype, out object result0, true)) { Exceptions.RaiseTypeError($"The Python function returned a tuple where element 0 was not {rtype} (the return type)"); - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } return result0; } @@ -380,7 +380,7 @@ private object TrueDispatch(object[] args) } string returnValueString = isVoid ? "" : "the return value and "; Exceptions.RaiseTypeError($"Expected a tuple ({sb}) of {returnValueString}the values for out and ref parameters, got {tpName}."); - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } } @@ -392,7 +392,7 @@ private object TrueDispatch(object[] args) object result; if (!Converter.ToManaged(op, rtype, out result, true)) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } return result; diff --git a/src/runtime/exceptions.cs b/src/runtime/exceptions.cs index 733a2ba80..2647a41c0 100644 --- a/src/runtime/exceptions.cs +++ b/src/runtime/exceptions.cs @@ -94,7 +94,7 @@ internal static Exception ToException(IntPtr ob) /// /// Readability of the Exceptions class improvements as we look toward version 2.7 ... /// - public static class Exceptions + internal static class Exceptions { internal static PyModule warnings_module; internal static PyModule exceptions_module; @@ -225,19 +225,6 @@ public static bool ExceptionMatches(IntPtr ob) return Runtime.PyErr_ExceptionMatches(ob) != 0; } - /// - /// ExceptionMatches Method - /// - /// - /// Returns true if the given Python exception matches the given - /// Python object. This is a wrapper for PyErr_GivenExceptionMatches. - /// - public static bool ExceptionMatches(IntPtr exc, IntPtr ob) - { - int i = Runtime.PyErr_GivenExceptionMatches(exc, ob); - return i != 0; - } - /// /// SetError Method /// @@ -271,7 +258,7 @@ public static void SetError(IntPtr type, IntPtr exceptionObject) /// object. The CLR exception instance is wrapped as a Python /// object, allowing it to be handled naturally from Python. /// - public static void SetError(Exception e) + public static bool SetError(Exception e) { // Because delegates allow arbitrary nesting of Python calling // managed calling Python calling... etc. it is possible that we @@ -282,32 +269,33 @@ public static void SetError(Exception e) if (pe != null) { pe.Restore(); - return; + return true; } - IntPtr op = Converter.ToPython(e); - IntPtr etype = Runtime.PyObject_GetAttr(op, PyIdentifier.__class__); + using var instance = Converter.ToPythonReference(e); + if (instance.IsNull()) return false; + var exceptionInfo = ExceptionDispatchInfo.Capture(e); - IntPtr pyInfo = Converter.ToPython(exceptionInfo); - Runtime.PyObject_SetAttrString(op, DispatchInfoAttribute, pyInfo); - Runtime.XDecref(pyInfo); + using var pyInfo = Converter.ToPythonReference(exceptionInfo); - Runtime.PyErr_SetObject(etype, op); - Runtime.XDecref(etype); - Runtime.XDecref(op); + if (Runtime.PyObject_SetAttrString(instance, DispatchInfoAttribute, pyInfo) != 0) + return false; + + var type = Runtime.PyObject_TYPE(instance); + Runtime.PyErr_SetObject(type, instance); + return true; } /// /// When called after SetError, sets the cause of the error. /// /// The cause of the current error - public static void SetCause(PythonException cause) + public static void SetCause(Exception cause) { - var currentException = new PythonException(); + var currentException = PythonException.FetchCurrentRaw(); currentException.Normalize(); - cause.Normalize(); - Runtime.XIncref(cause.PyValue); - Runtime.PyException_SetCause(currentException.PyValue, cause.PyValue); + using var causeInstance = Converter.ToPythonReference(cause); + Runtime.PyException_SetCause(currentException.Value.Reference, causeInstance); currentException.Restore(); } @@ -394,16 +382,19 @@ public static void deprecation(string message) /// IntPtr.Zero internal static IntPtr RaiseTypeError(string message) { - PythonException previousException = null; - if (ErrorOccurred()) - { - previousException = new PythonException(); - } + var cause = PythonException.FetchCurrentOrNullRaw(); + cause?.Normalize(); + Exceptions.SetError(Exceptions.TypeError, message); - if (previousException != null) - { - SetCause(previousException); - } + + if (cause is null) return IntPtr.Zero; + + var typeError = PythonException.FetchCurrentRaw(); + typeError.Normalize(); + + Runtime.PyException_SetCause(typeError.Value.Reference, cause.Value.Reference); + typeError.Restore(); + return IntPtr.Zero; } diff --git a/src/runtime/finalizer.cs b/src/runtime/finalizer.cs index fe2e46aac..afc0f8121 100644 --- a/src/runtime/finalizer.cs +++ b/src/runtime/finalizer.cs @@ -160,7 +160,7 @@ private void DisposeAll() { // Python requires finalizers to preserve exception: // https://docs.python.org/3/extending/newtypes.html#finalization-and-de-allocation - Runtime.PyErr_Restore(errType, errVal, traceback); + Runtime.PyErr_Restore(errType.Steal(), errVal.Steal(), traceback.Steal()); } } } diff --git a/src/runtime/importhook.cs b/src/runtime/importhook.cs index 184b588ad..cef8138ad 100644 --- a/src/runtime/importhook.cs +++ b/src/runtime/importhook.cs @@ -294,9 +294,7 @@ public static IntPtr __import__(IntPtr self, IntPtr argsRaw, IntPtr kw) return IntPtr.Zero; } // Save the exception - var originalException = new PythonException(); - // Otherwise, just clear the it. - Exceptions.Clear(); + var originalException = PythonException.FetchCurrentRaw(); string[] names = realname.Split('.'); diff --git a/src/runtime/methodbinder.cs b/src/runtime/methodbinder.cs index 8f74e0052..6b0976b97 100644 --- a/src/runtime/methodbinder.cs +++ b/src/runtime/methodbinder.cs @@ -438,7 +438,7 @@ internal Binding Bind(IntPtr inst, IntPtr args, IntPtr kw, MethodBase info, Meth outs: out outs); if (margs == null) { - mismatchedMethods.Add(new MismatchedMethod(new PythonException(), mi)); + mismatchedMethods.Add(new MismatchedMethod(PythonException.FetchCurrentRaw(), mi)); Exceptions.Clear(); continue; } diff --git a/src/runtime/moduleobject.cs b/src/runtime/moduleobject.cs index 41167e322..54761f6df 100644 --- a/src/runtime/moduleobject.cs +++ b/src/runtime/moduleobject.cs @@ -153,7 +153,7 @@ private void StoreAttribute(string name, ManagedType ob) { if (Runtime.PyDict_SetItemString(dict, name, ob.pyHandle) != 0) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } ob.IncrRefCount(); cache[name] = ob; @@ -351,7 +351,7 @@ protected override void OnSave(InterDomainContext context) } else if (Exceptions.ErrorOccurred()) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } pair.Value.DecrRefCount(); } diff --git a/src/runtime/pybuffer.cs b/src/runtime/pybuffer.cs index cf657a033..9fe22aca7 100644 --- a/src/runtime/pybuffer.cs +++ b/src/runtime/pybuffer.cs @@ -18,7 +18,7 @@ unsafe internal PyBuffer(PyObject exporter, PyBUF flags) if (Runtime.PyObject_GetBuffer(exporter.Handle, ref _view, (int)flags) < 0) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } _exporter = exporter; @@ -127,7 +127,7 @@ public void FromContiguous(IntPtr buf, long len, BufferOrderStyle fort) throw new NotSupportedException("FromContiguous requires at least Python 3.7"); if (Runtime.PyBuffer_FromContiguous(ref _view, buf, (IntPtr)len, OrderStyleToChar(fort, false)) < 0) - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } /// @@ -141,7 +141,7 @@ public void ToContiguous(IntPtr buf, BufferOrderStyle order) throw new ObjectDisposedException(nameof(PyBuffer)); if (Runtime.PyBuffer_ToContiguous(buf, ref _view, _view.len, OrderStyleToChar(order, true)) < 0) - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } /// @@ -167,7 +167,7 @@ public void FillInfo(IntPtr exporter, IntPtr buf, long len, bool _readonly, int if (disposedValue) throw new ObjectDisposedException(nameof(PyBuffer)); if (Runtime.PyBuffer_FillInfo(ref _view, exporter, buf, (IntPtr)len, Convert.ToInt32(_readonly), flags) < 0) - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } /// diff --git a/src/runtime/pyiter.cs b/src/runtime/pyiter.cs index 2016ef4f8..da2a600c6 100644 --- a/src/runtime/pyiter.cs +++ b/src/runtime/pyiter.cs @@ -57,7 +57,7 @@ public bool MoveNext() { if (Exceptions.ErrorOccurred()) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } // stop holding the previous object, if there was one diff --git a/src/runtime/pyobject.cs b/src/runtime/pyobject.cs index 7dbb1c690..72e61e759 100644 --- a/src/runtime/pyobject.cs +++ b/src/runtime/pyobject.cs @@ -29,9 +29,6 @@ public partial class PyObject : DynamicObject, IEnumerable, IDisposabl public static PyObject None => new PyObject(new BorrowedReference(Runtime.PyNone)); internal BorrowedReference Reference => new BorrowedReference(this.obj); - internal NewReference MakeNewReferenceOrNull() - => NewReference.DangerousFromPointer( - this.obj == IntPtr.Zero ? IntPtr.Zero : Runtime.SelfIncRef(this.obj)); /// /// PyObject Constructor @@ -82,6 +79,17 @@ internal PyObject(BorrowedReference reference) #endif } + internal PyObject(StolenReference reference) + { + if (reference == null) throw new ArgumentNullException(nameof(reference)); + + obj = reference.DangerousGetAddressOrNull(); + Finalizer.Instance.ThrottledCollect(); +#if TRACE_ALLOC + Traceback = new StackTrace(1); +#endif + } + // Ensure that encapsulated Python object is decref'ed appropriately // when the managed wrapper is garbage-collected. ~PyObject() @@ -147,7 +155,8 @@ public object AsManagedObject(Type t) object result; if (!Converter.ToManaged(obj, t, out result, true)) { - throw new InvalidCastException("cannot convert object to target type", new PythonException()); + throw new InvalidCastException("cannot convert object to target type", + PythonException.FetchCurrentOrNull(out _)); } return result; } @@ -637,7 +646,7 @@ public virtual long Length() var s = Runtime.PyObject_Size(Reference); if (s < 0) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } return s; } @@ -1455,4 +1464,13 @@ public override IEnumerable GetDynamicMemberNames() } } } + + internal static class PyObjectExtensions + { + internal static NewReference NewReferenceOrNull(this PyObject self) + => NewReference.DangerousFromPointer( + (self?.obj ?? IntPtr.Zero) == IntPtr.Zero + ? IntPtr.Zero + : Runtime.SelfIncRef(self.obj)); + } } diff --git a/src/runtime/pythonengine.cs b/src/runtime/pythonengine.cs index c5f5a65c4..52cc4e5e6 100644 --- a/src/runtime/pythonengine.cs +++ b/src/runtime/pythonengine.cs @@ -523,7 +523,7 @@ public static void Exec(string code, IntPtr? globals = null, IntPtr? locals = nu using PyObject result = RunString(code, globalsRef, localsRef, RunFlagType.File); if (result.obj != Runtime.PyNone) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } } /// @@ -776,7 +776,8 @@ public static void With(PyObject obj, Action Body) PyObject type = PyObject.None; PyObject val = PyObject.None; PyObject traceBack = PyObject.None; - PythonException ex = null; + Exception ex = null; + PythonException pyError = null; try { @@ -785,13 +786,20 @@ public static void With(PyObject obj, Action Body) Body(enterResult); } catch (PythonException e) + { + ex = pyError = e; + } + catch (Exception e) { ex = e; - type = ex.PyType ?? type; - val = ex.PyValue ?? val; - traceBack = ex.PyTB ?? traceBack; + Exceptions.SetError(e); + pyError = PythonException.FetchCurrentRaw(); } + type = pyError?.Type ?? type; + val = pyError?.Value ?? val; + traceBack = pyError?.Traceback ?? traceBack; + var exitResult = obj.InvokeMethod("__exit__", type, val, traceBack); if (ex != null && !exitResult.IsTrue()) throw ex; diff --git a/src/runtime/pythonexception.cs b/src/runtime/pythonexception.cs index f11f358d9..fdd22399d 100644 --- a/src/runtime/pythonexception.cs +++ b/src/runtime/pythonexception.cs @@ -12,49 +12,22 @@ namespace Python.Runtime /// public class PythonException : System.Exception { - private PyObject _type; - private PyObject _value; - private PyObject _pyTB; - private string _traceback = ""; - private string _message = ""; - private string _pythonTypeName = ""; - private bool disposed = false; - - [Obsolete("Please, use ThrowLastAsClrException or FromPyErr instead")] - public PythonException() - { - IntPtr gs = PythonEngine.AcquireLock(); - Runtime.PyErr_Fetch(out var type, out var value, out var traceback); - _type = type.MoveToPyObjectOrNull(); - _value = value.MoveToPyObjectOrNull(); - _pyTB = traceback.MoveToPyObjectOrNull(); - if (_type != null && _value != null) - { - using (PyObject pyTypeName = _type.GetAttr("__name__")) - { - _pythonTypeName = pyTypeName.ToString(); - } - _message = _pythonTypeName + " : " + _value; - } - if (_pyTB != null) - { - _traceback = TracebackToString(_pyTB); - } - PythonEngine.ReleaseLock(gs); + private PythonException(PyType type, PyObject value, PyObject traceback, + Exception innerException) + : base("An exception has occurred in Python code", innerException) + { + Type = type; + Value = value; + Traceback = traceback; } - private PythonException(PyObject type, PyObject value, PyObject traceback, - string message, string pythonTypeName, string tracebackText, - Exception innerException) - : base(message, innerException) + private PythonException(PyType type, PyObject value, PyObject traceback) + : base("An exception has occurred in Python code") { - _type = type; - _value = value; - _pyTB = traceback; - _message = message; - _pythonTypeName = pythonTypeName ?? _pythonTypeName; - _traceback = tracebackText ?? _traceback; + Type = type; + Value = value; + Traceback = traceback; } /// @@ -64,6 +37,47 @@ private PythonException(PyObject type, PyObject value, PyObject traceback, /// internal static Exception ThrowLastAsClrException() { + var exception = FetchCurrentOrNull(out ExceptionDispatchInfo dispatchInfo) + ?? throw new InvalidOperationException("No exception is set"); + dispatchInfo?.Throw(); + // when dispatchInfo is not null, this line will not be reached + throw exception; + } + + internal static PythonException FetchCurrentOrNullRaw() + { + IntPtr gs = PythonEngine.AcquireLock(); + try + { + Runtime.PyErr_Fetch(type: out var type, val: out var value, tb: out var traceback); + if (type.IsNull() && value.IsNull()) + return null; + + try + { + return new PythonException( + type: new PyType(type.Steal()), + value: value.MoveToPyObjectOrNull(), + traceback: traceback.MoveToPyObjectOrNull()); + } + finally + { + type.Dispose(); + } + } + finally + { + PythonEngine.ReleaseLock(gs); + } + } + internal static PythonException FetchCurrentRaw() + => FetchCurrentOrNullRaw() + ?? throw new InvalidOperationException("No exception is set"); + + internal static Exception FetchCurrentOrNull(out ExceptionDispatchInfo dispatchInfo) + { + dispatchInfo = default; + // prevent potential interop errors in this method // from crashing process with undebuggable StackOverflowException RuntimeHelpers.EnsureSufficientExecutionStack(); @@ -72,28 +86,27 @@ internal static Exception ThrowLastAsClrException() try { Runtime.PyErr_Fetch(out var type, out var value, out var traceback); + if (value.IsNull() && type.IsNull()) return null; + try { -#if NETSTANDARD if (!value.IsNull()) { - var exceptionInfo = TryGetDispatchInfo(value); - if (exceptionInfo != null) + dispatchInfo = TryGetDispatchInfo(value); + if (dispatchInfo != null) { - exceptionInfo.Throw(); - throw exceptionInfo.SourceException; // unreachable + return dispatchInfo.SourceException; } } -#endif var clrObject = ManagedType.GetManagedObject(value) as CLRObject; if (clrObject?.inst is Exception e) { - throw e; + return e; } var result = FromPyErr(type, value, traceback); - throw result; + return result; } finally { @@ -108,8 +121,7 @@ internal static Exception ThrowLastAsClrException() } } -#if NETSTANDARD - static ExceptionDispatchInfo TryGetDispatchInfo(BorrowedReference exception) + private static ExceptionDispatchInfo TryGetDispatchInfo(BorrowedReference exception) { if (exception.IsNull) return null; @@ -137,15 +149,13 @@ static ExceptionDispatchInfo TryGetDispatchInfo(BorrowedReference exception) pyInfo.Dispose(); } } -#endif /// /// Requires lock to be acquired elsewhere /// - static Exception FromPyErr(BorrowedReference typeHandle, BorrowedReference valueHandle, BorrowedReference tracebackHandle) + private static Exception FromPyErr(BorrowedReference typeHandle, BorrowedReference valueHandle, BorrowedReference tracebackHandle) { Exception inner = null; - string pythonTypeName = null, msg = "", tracebackText = null; var exceptionDispatchInfo = TryGetDispatchInfo(valueHandle); if (exceptionDispatchInfo != null) @@ -153,13 +163,13 @@ static Exception FromPyErr(BorrowedReference typeHandle, BorrowedReference value return exceptionDispatchInfo.SourceException; } - var clrObject = ManagedType.GetManagedObject(valueHandle) as CLRObject; - if (clrObject?.inst is Exception e) + if (valueHandle != null + && ManagedType.GetManagedObject(valueHandle) is CLRObject { inst: Exception e }) { return e; } - var type = PyObject.FromNullableReference(typeHandle); + var type = PyType.FromNullableReference(typeHandle); var value = PyObject.FromNullableReference(valueHandle); var traceback = PyObject.FromNullableReference(tracebackHandle); @@ -171,43 +181,57 @@ static Exception FromPyErr(BorrowedReference typeHandle, BorrowedReference value return decodedException; } - using (PyObject pyTypeName = type.GetAttr("__name__")) - { - pythonTypeName = pyTypeName.ToString(); - } + var raw = new PythonException(type, value, traceback); + raw.Normalize(); - var cause = value.GetAttr("__cause__", null); - if (cause != null && cause.Handle != Runtime.PyNone) + using var cause = Runtime.PyException_GetCause(raw.Value.Reference); + if (!cause.IsNull() && !cause.IsNone()) { - using (var innerTraceback = cause.GetAttr("__traceback__", null)) - { - inner = FromPyErr( - typeHandle: cause.GetPythonTypeReference(), - valueHandle: cause.Reference, - tracebackHandle: innerTraceback is null - ? BorrowedReference.Null - : innerTraceback.Reference); - } + using var innerTraceback = Runtime.PyException_GetTraceback(cause); + inner = FromPyErr( + typeHandle: Runtime.PyObject_TYPE(cause), + valueHandle: cause, + tracebackHandle: innerTraceback); } } - if (traceback != null) + + return new PythonException(type, value, traceback, inner); + } + + private string GetMessage() => GetMessage(Value, Type); + + private static string GetMessage(PyObject value, PyType type) + { + using var _ = new Py.GILState(); + if (value != null && !value.IsNone()) + { + return value.ToString(); + } + + if (type != null) { - tracebackText = TracebackToString(traceback); + return type.Name; } - return new PythonException(type, value, traceback, - msg, pythonTypeName, tracebackText, inner); + throw new ArgumentException("One of the values must not be null"); } - static string TracebackToString(PyObject traceback) + private static string TracebackToString(PyObject traceback) { if (traceback is null) { throw new ArgumentNullException(nameof(traceback)); - throw new ArgumentNullException(nameof(traceback)); } - _finalized = true; - Finalizer.Instance.AddFinalizedObject(this); + + using var tracebackModule = PyModule.Import("traceback"); + using var stackLines = new PyList(tracebackModule.InvokeMethod("format_tb", traceback)); + stackLines.Reverse(); + var result = new StringBuilder(); + foreach (PyObject stackLine in stackLines) + { + result.Append(stackLine); + } + return result.ToString(); } /// Restores python error. @@ -215,46 +239,27 @@ public void Restore() { IntPtr gs = PythonEngine.AcquireLock(); Runtime.PyErr_Restore( - _type.MakeNewReferenceOrNull().Steal(), - _value.MakeNewReferenceOrNull().Steal(), - _pyTB.MakeNewReferenceOrNull().Steal()); + Type.NewReferenceOrNull().Steal(), + Value.NewReferenceOrNull().Steal(), + Traceback.NewReferenceOrNull().Steal()); PythonEngine.ReleaseLock(gs); } /// - /// PyType Property - /// - /// /// Returns the exception type as a Python object. - /// - public PyObject PyType => _type; - - /// - /// PyValue Property /// - /// - /// Returns the exception value as a Python object. - /// - public PyObject PyValue => _value; + public PyType Type { get; private set; } /// - /// PyTB Property + /// Returns the exception value as a Python object. /// - /// - /// Returns the TraceBack as a Python object. - /// - public PyObject PyTB => _pyTB; + /// + public PyObject Value { get; private set; } - /// - /// Message Property - /// /// - /// A string representing the python exception message. + /// Returns the TraceBack as a Python object. /// - public override string Message - { - get { return _message; } - } + public PyObject Traceback { get; } /// /// StackTrace Property @@ -263,20 +268,14 @@ public override string Message /// A string representing the python exception stack trace. /// public override string StackTrace - { - get { return _tb + base.StackTrace; } - } + => (Traceback is null ? "" : TracebackToString(Traceback)) + + base.StackTrace; - /// - /// Python error type name. - /// - public string PythonTypeName - { - get { return _pythonTypeName; } - } + public override string Message => GetMessage(); /// - /// Replaces PyValue with an instance of PyType, if PyValue is not already an instance of PyType. + /// Replaces Value with an instance of Type, if Value is not already an instance of Type. + /// public void Normalize() { IntPtr gs = PythonEngine.AcquireLock(); @@ -284,7 +283,18 @@ public void Normalize() { if (Exceptions.ErrorOccurred()) throw new InvalidOperationException("Cannot normalize when an error is set"); // If an error is set and this PythonException is unnormalized, the error will be cleared and the PythonException will be replaced by a different error. - Runtime.PyErr_NormalizeException(ref _pyType, ref _pyValue, ref _pyTB); + NewReference value = Value.NewReferenceOrNull(); + NewReference type = Type.NewReferenceOrNull(); + NewReference tb = Traceback.NewReferenceOrNull(); + Runtime.PyErr_NormalizeException(type: ref type, val: ref value, tb: ref tb); + Value = value.MoveToPyObject(); + Type = new PyType(type.Steal()); + if (!tb.IsNull()) + { + int r = Runtime.PyException_SetTraceback(Value.Reference, tb); + ThrowIfIsNotZero(r); + } + tb.Dispose(); } finally { @@ -302,30 +312,19 @@ public string Format() IntPtr gs = PythonEngine.AcquireLock(); try { - if (_pyTB != null && _type != null && _value != null) + var copy = Clone(); + copy.Normalize(); + + if (copy.Traceback != null && copy.Type != null && copy.Value != null) { - Runtime.XIncref(_pyType); - Runtime.XIncref(_pyValue); - Runtime.XIncref(_pyTB); - using (PyObject pyType = new PyObject(_pyType)) - using (PyObject pyValue = new PyObject(_pyValue)) - using (PyObject pyTB = new PyObject(_pyTB)) - using (PyObject tb_mod = PythonEngine.ImportModule("traceback")) + using var traceback = PyModule.Import("traceback"); + var buffer = new StringBuilder(); + var values = traceback.InvokeMethod("format_exception", copy.Type, copy.Value, copy.Traceback); + foreach (PyObject val in values) { - var buffer = new StringBuilder(); - var values = tb_mod.InvokeMethod("format_exception", _type, _value, _pyTB); - foreach (PyObject val in values) - { - buffer.Append(val.ToString()); - } - res = buffer.ToString(); - var values = tb_mod.InvokeMethod("format_exception", pyType, pyValue, pyTB); - foreach (PyObject val in values) - { - buffer.Append(val.ToString()); - } - res = buffer.ToString(); + buffer.Append(val); } + res = buffer.ToString(); } else { @@ -339,45 +338,36 @@ public string Format() return res; } - public bool IsMatches(IntPtr exc) + public PythonException Clone() + => new PythonException(Type, Value, Traceback, InnerException); + + internal bool Is(IntPtr type) { - return Runtime.PyErr_GivenExceptionMatches(PyType, exc) != 0; + return Runtime.PyErr_GivenExceptionMatches( + (Value ?? Type).Reference, + new BorrowedReference(type)) != 0; } /// - /// Dispose Method + /// Returns true if the current Python exception + /// matches the given exception type. /// - /// - /// The Dispose method provides a way to explicitly release the - /// Python objects represented by a PythonException. - /// If object not properly disposed can cause AppDomain unload issue. - /// See GH#397 and GH#400. - /// - public void Dispose() + internal static bool CurrentMatches(IntPtr ob) { - if (!disposed) - { - _type?.Dispose(); - _value?.Dispose(); - _pyTB?.Dispose(); - GC.SuppressFinalize(this); - disposed = true; - } + return Runtime.PyErr_ExceptionMatches(ob) != 0; } - /// - /// Matches Method - /// - /// - /// Returns true if the Python exception type represented by the - /// PythonException instance matches the given exception type. - /// - internal static bool Matches(IntPtr ob) + internal static BorrowedReference ThrowIfIsNull(BorrowedReference ob) { - return Runtime.PyErr_ExceptionMatches(ob) != 0; + if (ob == null) + { + throw ThrowLastAsClrException(); + } + + return ob; } - public static void ThrowIfIsNull(IntPtr ob) + public static IntPtr ThrowIfIsNull(IntPtr ob) { if (ob == IntPtr.Zero) { diff --git a/src/runtime/pytuple.cs b/src/runtime/pytuple.cs index 530ced3d2..5a18b6bed 100644 --- a/src/runtime/pytuple.cs +++ b/src/runtime/pytuple.cs @@ -77,7 +77,7 @@ private static IntPtr FromArray(PyObject[] items) if (res != 0) { Runtime.Py_DecRef(val); - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } } return val; diff --git a/src/runtime/pytype.cs b/src/runtime/pytype.cs index 8bc08b76d..85ee54171 100644 --- a/src/runtime/pytype.cs +++ b/src/runtime/pytype.cs @@ -13,6 +13,27 @@ public PyType(TypeSpec spec, PyTuple? bases = null) : base(FromSpec(spec, bases) /// Wraps an existing type object. public PyType(PyObject o) : base(FromObject(o)) { } + internal PyType(StolenReference reference) : base(EnsureIsType(in reference)) + { + } + + internal new static PyType? FromNullableReference(BorrowedReference reference) + => reference == null + ? null + : new PyType(new NewReference(reference).Steal()); + + public string Name + { + get + { + var namePtr = new StrPtr + { + RawPointer = Marshal.ReadIntPtr(Handle, TypeOffset.tp_name), + }; + return namePtr.ToString(System.Text.Encoding.UTF8)!; + } + } + /// Checks if specified object is a Python type. public static bool IsType(PyObject value) { @@ -21,6 +42,19 @@ public static bool IsType(PyObject value) return Runtime.PyType_Check(value.obj); } + private static IntPtr EnsureIsType(in StolenReference reference) + { + IntPtr address = reference.DangerousGetAddressOrNull(); + if (address == IntPtr.Zero) + throw new ArgumentNullException(nameof(reference)); + return EnsureIsType(address); + } + + private static IntPtr EnsureIsType(IntPtr ob) + => Runtime.PyType_Check(ob) + ? ob + : throw new ArgumentException("object is not a type"); + private static BorrowedReference FromObject(PyObject o) { if (o is null) throw new ArgumentNullException(nameof(o)); diff --git a/src/runtime/runtime.cs b/src/runtime/runtime.cs index 3ce000b5b..194e928c0 100644 --- a/src/runtime/runtime.cs +++ b/src/runtime/runtime.cs @@ -426,11 +426,10 @@ private static void RunExitFuncs() } catch (PythonException e) { - if (!e.IsMatches(Exceptions.ImportError)) + if (!e.Is(Exceptions.ImportError)) { throw; } - e.Dispose(); // The runtime may not provided `atexit` module. return; } @@ -443,7 +442,6 @@ private static void RunExitFuncs() catch (PythonException e) { Console.Error.WriteLine(e); - e.Dispose(); } } } @@ -492,9 +490,9 @@ private static void PyDictTryDelItem(BorrowedReference dict, string key) { return; } - if (!PythonException.Matches(Exceptions.KeyError)) + if (!PythonException.CurrentMatches(Exceptions.KeyError)) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } PyErr_Clear(); } @@ -1045,6 +1043,11 @@ internal static int PyObject_SetAttrString(IntPtr pointer, string name, IntPtr v using var namePtr = new StrPtr(name, Encoding.UTF8); return Delegates.PyObject_SetAttrString(pointer, namePtr, value); } + internal static int PyObject_SetAttrString(BorrowedReference @object, string name, BorrowedReference value) + { + using var namePtr = new StrPtr(name, Encoding.UTF8); + return Delegates.PyObject_SetAttrString(@object.DangerousGetAddress(), namePtr, value.DangerousGetAddress()); + } internal static int PyObject_HasAttr(BorrowedReference pointer, BorrowedReference name) => Delegates.PyObject_HasAttr(pointer, name); @@ -2085,19 +2088,19 @@ internal static void PyErr_SetString(IntPtr ob, string message) internal static int PyErr_ExceptionMatches(IntPtr exception) => Delegates.PyErr_ExceptionMatches(exception); - internal static int PyErr_GivenExceptionMatches(IntPtr ob, IntPtr val) => Delegates.PyErr_GivenExceptionMatches(ob, val); + internal static int PyErr_GivenExceptionMatches(BorrowedReference ob, BorrowedReference val) => Delegates.PyErr_GivenExceptionMatches(ob, val); - internal static void PyErr_NormalizeException(ref IntPtr ob, ref IntPtr val, ref IntPtr tb) => Delegates.PyErr_NormalizeException(ref ob, ref val, ref tb); + internal static void PyErr_NormalizeException(ref NewReference type, ref NewReference val, ref NewReference tb) => Delegates.PyErr_NormalizeException(ref type, ref val, ref tb); internal static IntPtr PyErr_Occurred() => Delegates.PyErr_Occurred(); - internal static void PyErr_Fetch(out NewReference ob, out NewReference val, out NewReference tb) => Delegates.PyErr_Fetch(out ob, out val, out tb); + internal static void PyErr_Fetch(out NewReference type, out NewReference val, out NewReference tb) => Delegates.PyErr_Fetch(out type, out val, out tb); - internal static void PyErr_Restore(IntPtr ob, IntPtr val, IntPtr tb) => Delegates.PyErr_Restore(ob, val, tb); + internal static void PyErr_Restore(StolenReference ob, StolenReference val, StolenReference tb) => Delegates.PyErr_Restore(ob, val, tb); internal static void PyErr_Clear() => Delegates.PyErr_Clear(); @@ -2105,11 +2108,19 @@ internal static void PyErr_SetString(IntPtr ob, string message) internal static void PyErr_Print() => Delegates.PyErr_Print(); + + internal static NewReference PyException_GetCause(BorrowedReference ex) + => Delegates.PyException_GetCause(ex); + internal static NewReference PyException_GetTraceback(BorrowedReference ex) + => Delegates.PyException_GetTraceback(ex); + /// /// Set the cause associated with the exception to cause. Use NULL to clear it. There is no type check to make sure that cause is either an exception instance or None. This steals a reference to cause. /// - - internal static void PyException_SetCause(IntPtr ex, IntPtr cause) => Delegates.PyException_SetCause(ex, cause); + internal static void PyException_SetCause(BorrowedReference ex, BorrowedReference cause) + => Delegates.PyException_SetCause(ex, cause); + internal static int PyException_SetTraceback(BorrowedReference ex, BorrowedReference tb) + => Delegates.PyException_SetTraceback(ex, tb); //==================================================================== // Cell API @@ -2487,11 +2498,11 @@ static Delegates() PyErr_SetFromErrno = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyErr_SetFromErrno), GetUnmanagedDll(_PythonDll)); PyErr_SetNone = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyErr_SetNone), GetUnmanagedDll(_PythonDll)); PyErr_ExceptionMatches = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyErr_ExceptionMatches), GetUnmanagedDll(_PythonDll)); - PyErr_GivenExceptionMatches = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyErr_GivenExceptionMatches), GetUnmanagedDll(_PythonDll)); - PyErr_NormalizeException = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyErr_NormalizeException), GetUnmanagedDll(_PythonDll)); + PyErr_GivenExceptionMatches = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyErr_GivenExceptionMatches), GetUnmanagedDll(_PythonDll)); + PyErr_NormalizeException = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyErr_NormalizeException), GetUnmanagedDll(_PythonDll)); PyErr_Occurred = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyErr_Occurred), GetUnmanagedDll(_PythonDll)); PyErr_Fetch = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyErr_Fetch), GetUnmanagedDll(_PythonDll)); - PyErr_Restore = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyErr_Restore), GetUnmanagedDll(_PythonDll)); + PyErr_Restore = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyErr_Restore), GetUnmanagedDll(_PythonDll)); PyErr_Clear = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyErr_Clear), GetUnmanagedDll(_PythonDll)); PyErr_Print = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyErr_Print), GetUnmanagedDll(_PythonDll)); PyCell_Get = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyCell_Get), GetUnmanagedDll(_PythonDll)); @@ -2508,7 +2519,10 @@ static Delegates() PyLong_AsSignedSize_t = (delegate* unmanaged[Cdecl])GetFunctionByName("PyLong_AsSsize_t", GetUnmanagedDll(_PythonDll)); PyExplicitlyConvertToInt64 = (delegate* unmanaged[Cdecl])GetFunctionByName("PyLong_AsLongLong", GetUnmanagedDll(_PythonDll)); PyDict_GetItemWithError = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyDict_GetItemWithError), GetUnmanagedDll(_PythonDll)); - PyException_SetCause = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyException_SetCause), GetUnmanagedDll(_PythonDll)); + PyException_GetCause = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyException_GetCause), GetUnmanagedDll(_PythonDll)); + PyException_GetTraceback = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyException_GetTraceback), GetUnmanagedDll(_PythonDll)); + PyException_SetCause = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyException_SetCause), GetUnmanagedDll(_PythonDll)); + PyException_SetTraceback = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyException_SetTraceback), GetUnmanagedDll(_PythonDll)); PyThreadState_SetAsyncExcLLP64 = (delegate* unmanaged[Cdecl])GetFunctionByName("PyThreadState_SetAsyncExc", GetUnmanagedDll(_PythonDll)); PyThreadState_SetAsyncExcLP64 = (delegate* unmanaged[Cdecl])GetFunctionByName("PyThreadState_SetAsyncExc", GetUnmanagedDll(_PythonDll)); PyType_FromSpecWithBases = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyType_FromSpecWithBases), GetUnmanagedDll(PythonDLL)); @@ -2764,11 +2778,11 @@ static Delegates() internal static delegate* unmanaged[Cdecl] PyErr_SetFromErrno { get; } internal static delegate* unmanaged[Cdecl] PyErr_SetNone { get; } internal static delegate* unmanaged[Cdecl] PyErr_ExceptionMatches { get; } - internal static delegate* unmanaged[Cdecl] PyErr_GivenExceptionMatches { get; } - internal static delegate* unmanaged[Cdecl] PyErr_NormalizeException { get; } + internal static delegate* unmanaged[Cdecl] PyErr_GivenExceptionMatches { get; } + internal static delegate* unmanaged[Cdecl] PyErr_NormalizeException { get; } internal static delegate* unmanaged[Cdecl] PyErr_Occurred { get; } internal static delegate* unmanaged[Cdecl] PyErr_Fetch { get; } - internal static delegate* unmanaged[Cdecl] PyErr_Restore { get; } + internal static delegate* unmanaged[Cdecl] PyErr_Restore { get; } internal static delegate* unmanaged[Cdecl] PyErr_Clear { get; } internal static delegate* unmanaged[Cdecl] PyErr_Print { get; } internal static delegate* unmanaged[Cdecl] PyCell_Get { get; } @@ -2785,7 +2799,10 @@ static Delegates() internal static delegate* unmanaged[Cdecl] PyLong_AsSignedSize_t { get; } internal static delegate* unmanaged[Cdecl] PyExplicitlyConvertToInt64 { get; } internal static delegate* unmanaged[Cdecl] PyDict_GetItemWithError { get; } - internal static delegate* unmanaged[Cdecl] PyException_SetCause { get; } + internal static delegate* unmanaged[Cdecl] PyException_GetCause { get; } + internal static delegate* unmanaged[Cdecl] PyException_GetTraceback { get; } + internal static delegate* unmanaged[Cdecl] PyException_SetCause { get; } + internal static delegate* unmanaged[Cdecl] PyException_SetTraceback { get; } internal static delegate* unmanaged[Cdecl] PyThreadState_SetAsyncExcLLP64 { get; } internal static delegate* unmanaged[Cdecl] PyThreadState_SetAsyncExcLP64 { get; } internal static delegate* unmanaged[Cdecl] PyObject_GenericGetDict { get; } diff --git a/src/runtime/typemanager.cs b/src/runtime/typemanager.cs index 01aceb656..fde8fb719 100644 --- a/src/runtime/typemanager.cs +++ b/src/runtime/typemanager.cs @@ -177,7 +177,7 @@ internal static IntPtr CreateType(Type impl) if (Runtime.PyType_Ready(type) != 0) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } var dict = new BorrowedReference(Marshal.ReadIntPtr(type, TypeOffset.tp_dict)); @@ -300,7 +300,7 @@ internal static IntPtr CreateType(ManagedType impl, Type clrType) if (Runtime.PyType_Ready(type) != 0) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } var dict = new BorrowedReference(Marshal.ReadIntPtr(type, TypeOffset.tp_dict)); @@ -472,7 +472,7 @@ internal static IntPtr CreateMetaType(Type impl, out SlotsHolder slotsHolder) if (Runtime.PyType_Ready(type) != 0) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } IntPtr dict = Marshal.ReadIntPtr(type, TypeOffset.tp_dict); @@ -577,7 +577,7 @@ internal static IntPtr BasicSubType(string name, IntPtr base_, Type impl) if (Runtime.PyType_Ready(type) != 0) { - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } IntPtr tp_dict = Marshal.ReadIntPtr(type, TypeOffset.tp_dict); @@ -926,14 +926,14 @@ public static IntPtr CreateObjectType() if (Runtime.PyDict_SetItemString(globals, "__builtins__", Runtime.PyEval_GetBuiltins()) != 0) { globals.Dispose(); - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } const string code = "class A(object): pass"; using var resRef = Runtime.PyRun_String(code, RunFlagType.File, globals, globals); if (resRef.IsNull()) { globals.Dispose(); - throw new PythonException(); + throw PythonException.ThrowLastAsClrException(); } resRef.Dispose(); BorrowedReference A = Runtime.PyDict_GetItemString(globals, "A"); From e58411d90547cb3da22697c09edb6f190e0469ad Mon Sep 17 00:00:00 2001 From: Victor Nova Date: Fri, 9 Apr 2021 12:08:05 -0700 Subject: [PATCH 15/39] rum embedding tests before Python tests --- .github/workflows/main.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 2dd75c529..0afb6016a 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -56,6 +56,9 @@ jobs: run: | python -m pythonnet.find_libpython --export | Out-File -FilePath $env:GITHUB_ENV -Encoding utf8 -Append + - name: Embedding tests + run: dotnet test --runtime any-${{ matrix.platform }} src/embed_tests/ + - name: Python Tests (Mono) if: ${{ matrix.os != 'windows' }} run: pytest --runtime mono @@ -67,9 +70,6 @@ jobs: if: ${{ matrix.os == 'windows' }} run: pytest --runtime netfx - - name: Embedding tests - run: dotnet test --runtime any-${{ matrix.platform }} src/embed_tests/ - - name: Python tests run from .NET run: dotnet test --runtime any-${{ matrix.platform }} src/python_tests_runner/ From 00653dcf9a72b814a45a47588750f4ef32fb5497 Mon Sep 17 00:00:00 2001 From: Victor Nova Date: Fri, 9 Apr 2021 12:40:58 -0700 Subject: [PATCH 16/39] PythonException.StackTrace is GIL-safe --- src/runtime/pythonexception.cs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/runtime/pythonexception.cs b/src/runtime/pythonexception.cs index fdd22399d..0912b828e 100644 --- a/src/runtime/pythonexception.cs +++ b/src/runtime/pythonexception.cs @@ -268,8 +268,15 @@ public void Restore() /// A string representing the python exception stack trace. /// public override string StackTrace - => (Traceback is null ? "" : TracebackToString(Traceback)) - + base.StackTrace; + { + get + { + if (Traceback is null) return base.StackTrace; + + using var _ = new Py.GILState(); + return TracebackToString(Traceback) + base.StackTrace; + } + } public override string Message => GetMessage(); From 343320139f410534d5f8f5d80dec99e24b9088e3 Mon Sep 17 00:00:00 2001 From: Victor Nova Date: Sat, 10 Apr 2021 17:34:30 -0700 Subject: [PATCH 17/39] separate .Steal() and .StealNullable() --- src/runtime/NewReference.cs | 16 +++++++++++++++- src/runtime/finalizer.cs | 2 +- src/runtime/pyobject.cs | 2 +- src/runtime/pythonexception.cs | 6 +++--- 4 files changed, 20 insertions(+), 6 deletions(-) diff --git a/src/runtime/NewReference.cs b/src/runtime/NewReference.cs index 3bc5d9724..c037f988f 100644 --- a/src/runtime/NewReference.cs +++ b/src/runtime/NewReference.cs @@ -2,6 +2,7 @@ namespace Python.Runtime { using System; using System.Diagnostics.Contracts; + using System.Runtime.CompilerServices; /// /// Represents a reference to a Python object, that is tracked by Python's reference counting. @@ -65,12 +66,25 @@ public IntPtr DangerousMoveToPointerOrNull() /// Call this method to move ownership of this reference to a Python C API function, /// that steals reference passed to it. /// - public StolenReference Steal() + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public StolenReference StealNullable() { IntPtr rawPointer = this.pointer; this.pointer = IntPtr.Zero; return new StolenReference(rawPointer); } + + /// + /// Call this method to move ownership of this reference to a Python C API function, + /// that steals reference passed to it. + /// + [MethodImpl(MethodImplOptions.AggressiveInlining)] + public StolenReference Steal() + { + if (this.IsNull()) throw new NullReferenceException(); + + return this.StealNullable(); + } /// /// Removes this reference to a Python object, and sets it to null. /// diff --git a/src/runtime/finalizer.cs b/src/runtime/finalizer.cs index afc0f8121..91a4b859e 100644 --- a/src/runtime/finalizer.cs +++ b/src/runtime/finalizer.cs @@ -160,7 +160,7 @@ private void DisposeAll() { // Python requires finalizers to preserve exception: // https://docs.python.org/3/extending/newtypes.html#finalization-and-de-allocation - Runtime.PyErr_Restore(errType.Steal(), errVal.Steal(), traceback.Steal()); + Runtime.PyErr_Restore(errType.StealNullable(), errVal.StealNullable(), traceback.StealNullable()); } } } diff --git a/src/runtime/pyobject.cs b/src/runtime/pyobject.cs index 72e61e759..65dea3665 100644 --- a/src/runtime/pyobject.cs +++ b/src/runtime/pyobject.cs @@ -218,7 +218,7 @@ protected virtual void Dispose(bool disposing) { // Python requires finalizers to preserve exception: // https://docs.python.org/3/extending/newtypes.html#finalization-and-de-allocation - Runtime.PyErr_Restore(errType.Steal(), errVal.Steal(), traceback.Steal()); + Runtime.PyErr_Restore(errType.StealNullable(), errVal.StealNullable(), traceback.StealNullable()); } } else diff --git a/src/runtime/pythonexception.cs b/src/runtime/pythonexception.cs index 0912b828e..d80fbb195 100644 --- a/src/runtime/pythonexception.cs +++ b/src/runtime/pythonexception.cs @@ -239,9 +239,9 @@ public void Restore() { IntPtr gs = PythonEngine.AcquireLock(); Runtime.PyErr_Restore( - Type.NewReferenceOrNull().Steal(), - Value.NewReferenceOrNull().Steal(), - Traceback.NewReferenceOrNull().Steal()); + Type.NewReferenceOrNull().StealNullable(), + Value.NewReferenceOrNull().StealNullable(), + Traceback.NewReferenceOrNull().StealNullable()); PythonEngine.ReleaseLock(gs); } From 95cc52fdb221909f8b93e0382a979417ef82d4f8 Mon Sep 17 00:00:00 2001 From: Victor Nova Date: Sat, 10 Apr 2021 17:34:52 -0700 Subject: [PATCH 18/39] can't test exception type when runtime is down --- src/embed_tests/pyinitialize.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/embed_tests/pyinitialize.cs b/src/embed_tests/pyinitialize.cs index 1683aeca1..1622f46d3 100644 --- a/src/embed_tests/pyinitialize.cs +++ b/src/embed_tests/pyinitialize.cs @@ -158,9 +158,10 @@ public static void TestRunExitFuncs() catch (PythonException e) { string msg = e.ToString(); + bool isImportError = e.Is(Exceptions.ImportError); Runtime.Runtime.Shutdown(); - if (e.Is(Exceptions.ImportError)) + if (isImportError) { Assert.Ignore("no atexit module"); } From 63ad42ce7e91130af178a687a6c4a87d08344035 Mon Sep 17 00:00:00 2001 From: Victor Nova Date: Sat, 10 Apr 2021 17:35:50 -0700 Subject: [PATCH 19/39] PythonException: dispose intermediate values used in stack trace generation --- src/runtime/pythonexception.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/runtime/pythonexception.cs b/src/runtime/pythonexception.cs index d80fbb195..35ff84dc2 100644 --- a/src/runtime/pythonexception.cs +++ b/src/runtime/pythonexception.cs @@ -230,6 +230,7 @@ private static string TracebackToString(PyObject traceback) foreach (PyObject stackLine in stackLines) { result.Append(stackLine); + stackLine.Dispose(); } return result.ToString(); } @@ -326,10 +327,11 @@ public string Format() { using var traceback = PyModule.Import("traceback"); var buffer = new StringBuilder(); - var values = traceback.InvokeMethod("format_exception", copy.Type, copy.Value, copy.Traceback); + using var values = traceback.InvokeMethod("format_exception", copy.Type, copy.Value, copy.Traceback); foreach (PyObject val in values) { buffer.Append(val); + val.Dispose(); } res = buffer.ToString(); } From faec7fc1635cc14416eae49101b2bfb62f3536f4 Mon Sep 17 00:00:00 2001 From: Victor Nova Date: Sat, 10 Apr 2021 17:36:13 -0700 Subject: [PATCH 20/39] Point users to Message property of PythonException --- src/runtime/pythonexception.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/runtime/pythonexception.cs b/src/runtime/pythonexception.cs index 35ff84dc2..47db8ad2c 100644 --- a/src/runtime/pythonexception.cs +++ b/src/runtime/pythonexception.cs @@ -15,7 +15,7 @@ public class PythonException : System.Exception private PythonException(PyType type, PyObject value, PyObject traceback, Exception innerException) - : base("An exception has occurred in Python code", innerException) + : base("An exception has occurred in Python code. See Message property for details.", innerException) { Type = type; Value = value; @@ -23,7 +23,7 @@ private PythonException(PyType type, PyObject value, PyObject traceback, } private PythonException(PyType type, PyObject value, PyObject traceback) - : base("An exception has occurred in Python code") + : base("An exception has occurred in Python code. See Message property for details.") { Type = type; Value = value; From dfc70f682287b0c68d7160d4b37df2361c02bb35 Mon Sep 17 00:00:00 2001 From: Victor Nova Date: Sat, 10 Apr 2021 19:33:49 -0700 Subject: [PATCH 21/39] minor change in PythonEngine.With --- src/runtime/pythonengine.cs | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/runtime/pythonengine.cs b/src/runtime/pythonengine.cs index 52cc4e5e6..419d4554a 100644 --- a/src/runtime/pythonengine.cs +++ b/src/runtime/pythonengine.cs @@ -773,9 +773,6 @@ public static void With(PyObject obj, Action Body) // Behavior described here: // https://docs.python.org/2/reference/datamodel.html#with-statement-context-managers - PyObject type = PyObject.None; - PyObject val = PyObject.None; - PyObject traceBack = PyObject.None; Exception ex = null; PythonException pyError = null; @@ -796,9 +793,9 @@ public static void With(PyObject obj, Action Body) pyError = PythonException.FetchCurrentRaw(); } - type = pyError?.Type ?? type; - val = pyError?.Value ?? val; - traceBack = pyError?.Traceback ?? traceBack; + PyObject type = pyError?.Type ?? PyObject.None; + PyObject val = pyError?.Value ?? PyObject.None; + PyObject traceBack = pyError?.Traceback ?? PyObject.None; var exitResult = obj.InvokeMethod("__exit__", type, val, traceBack); From d976acf44de2e6d1c67929522b25b7528c747501 Mon Sep 17 00:00:00 2001 From: Victor Nova Date: Sat, 10 Apr 2021 19:37:37 -0700 Subject: [PATCH 22/39] simplified code of PythonException and added a lot more checks --- src/runtime/exceptions.cs | 2 +- src/runtime/pythonexception.cs | 287 +++++++++++++++++---------------- src/runtime/pytype.cs | 3 + src/runtime/runtime.cs | 19 +-- 4 files changed, 159 insertions(+), 152 deletions(-) diff --git a/src/runtime/exceptions.cs b/src/runtime/exceptions.cs index 2647a41c0..669ef9e6f 100644 --- a/src/runtime/exceptions.cs +++ b/src/runtime/exceptions.cs @@ -308,7 +308,7 @@ public static void SetCause(Exception cause) /// public static bool ErrorOccurred() { - return Runtime.PyErr_Occurred() != IntPtr.Zero; + return Runtime.PyErr_Occurred() != null; } /// diff --git a/src/runtime/pythonexception.cs b/src/runtime/pythonexception.cs index 47db8ad2c..3f4bd8154 100644 --- a/src/runtime/pythonexception.cs +++ b/src/runtime/pythonexception.cs @@ -1,4 +1,6 @@ +#nullable enable using System; +using System.Diagnostics; using System.Linq; using System.Runtime.CompilerServices; using System.Runtime.ExceptionServices; @@ -13,22 +15,17 @@ namespace Python.Runtime public class PythonException : System.Exception { - private PythonException(PyType type, PyObject value, PyObject traceback, - Exception innerException) + public PythonException(PyType type, PyObject? value, PyObject? traceback, + Exception? innerException) : base("An exception has occurred in Python code. See Message property for details.", innerException) { - Type = type; + Type = type ?? throw new ArgumentNullException(nameof(type)); Value = value; Traceback = traceback; } - private PythonException(PyType type, PyObject value, PyObject traceback) - : base("An exception has occurred in Python code. See Message property for details.") - { - Type = type; - Value = value; - Traceback = traceback; - } + public PythonException(PyType type, PyObject? value, PyObject? traceback) + : this(type, value, traceback, innerException: null) { } /// /// Rethrows the last Python exception as corresponding CLR exception. @@ -37,91 +34,67 @@ private PythonException(PyType type, PyObject value, PyObject traceback) /// internal static Exception ThrowLastAsClrException() { - var exception = FetchCurrentOrNull(out ExceptionDispatchInfo dispatchInfo) + var exception = FetchCurrentOrNull(out ExceptionDispatchInfo? dispatchInfo) ?? throw new InvalidOperationException("No exception is set"); dispatchInfo?.Throw(); // when dispatchInfo is not null, this line will not be reached throw exception; } - internal static PythonException FetchCurrentOrNullRaw() + internal static PythonException? FetchCurrentOrNullRaw() { - IntPtr gs = PythonEngine.AcquireLock(); - try - { - Runtime.PyErr_Fetch(type: out var type, val: out var value, tb: out var traceback); - if (type.IsNull() && value.IsNull()) - return null; + using var _ = new Py.GILState(); - try - { - return new PythonException( - type: new PyType(type.Steal()), - value: value.MoveToPyObjectOrNull(), - traceback: traceback.MoveToPyObjectOrNull()); - } - finally - { - type.Dispose(); - } - } - finally + Runtime.PyErr_Fetch(type: out var type, val: out var value, tb: out var traceback); + + if (type.IsNull()) { - PythonEngine.ReleaseLock(gs); + Debug.Assert(value.IsNull()); + Debug.Assert(traceback.IsNull()); + return null; } + + return new PythonException( + type: new PyType(type.Steal()), + value: value.MoveToPyObjectOrNull(), + traceback: traceback.MoveToPyObjectOrNull()); } internal static PythonException FetchCurrentRaw() => FetchCurrentOrNullRaw() ?? throw new InvalidOperationException("No exception is set"); - internal static Exception FetchCurrentOrNull(out ExceptionDispatchInfo dispatchInfo) + internal static Exception? FetchCurrentOrNull(out ExceptionDispatchInfo? dispatchInfo) { - dispatchInfo = default; + dispatchInfo = null; // prevent potential interop errors in this method // from crashing process with undebuggable StackOverflowException RuntimeHelpers.EnsureSufficientExecutionStack(); - IntPtr gs = PythonEngine.AcquireLock(); - try + using var _ = new Py.GILState(); + Runtime.PyErr_Fetch(out var type, out var value, out var traceback); + if (type.IsNull()) { - Runtime.PyErr_Fetch(out var type, out var value, out var traceback); - if (value.IsNull() && type.IsNull()) return null; - - try - { - if (!value.IsNull()) - { - dispatchInfo = TryGetDispatchInfo(value); - if (dispatchInfo != null) - { - return dispatchInfo.SourceException; - } - } + Debug.Assert(value.IsNull()); + Debug.Assert(traceback.IsNull()); + return null; + } - var clrObject = ManagedType.GetManagedObject(value) as CLRObject; - if (clrObject?.inst is Exception e) - { - return e; - } + Runtime.PyErr_NormalizeException(type: ref type, val: ref value, tb: ref traceback); - var result = FromPyErr(type, value, traceback); - return result; - } - finally - { - type.Dispose(); - value.Dispose(); - traceback.Dispose(); - } + try + { + return FromPyErr(typeRef: type, valRef: value, tbRef: traceback, out dispatchInfo); } finally { - PythonEngine.ReleaseLock(gs); + type.Dispose(); + value.Dispose(); + traceback.Dispose(); } } - private static ExceptionDispatchInfo TryGetDispatchInfo(BorrowedReference exception) + private static ExceptionDispatchInfo? TryGetDispatchInfo(BorrowedReference exception) { if (exception.IsNull) return null; @@ -153,54 +126,53 @@ private static ExceptionDispatchInfo TryGetDispatchInfo(BorrowedReference except /// /// Requires lock to be acquired elsewhere /// - private static Exception FromPyErr(BorrowedReference typeHandle, BorrowedReference valueHandle, BorrowedReference tracebackHandle) + private static Exception FromPyErr(BorrowedReference typeRef, BorrowedReference valRef, BorrowedReference tbRef, + out ExceptionDispatchInfo? exceptionDispatchInfo) { - Exception inner = null; + if (valRef == null) throw new ArgumentNullException(nameof(valRef)); + + var type = PyType.FromReference(typeRef); + var value = new PyObject(valRef); + var traceback = PyObject.FromNullableReference(tbRef); - var exceptionDispatchInfo = TryGetDispatchInfo(valueHandle); + exceptionDispatchInfo = TryGetDispatchInfo(valRef); if (exceptionDispatchInfo != null) { return exceptionDispatchInfo.SourceException; } - if (valueHandle != null - && ManagedType.GetManagedObject(valueHandle) is CLRObject { inst: Exception e }) + if (ManagedType.GetManagedObject(valRef) is CLRObject { inst: Exception e }) { return e; } - var type = PyType.FromNullableReference(typeHandle); - var value = PyObject.FromNullableReference(valueHandle); - var traceback = PyObject.FromNullableReference(tracebackHandle); - - if (type != null && value != null) + if (PyObjectConversions.TryDecode(valRef, typeRef, typeof(Exception), out object decoded) + && decoded is Exception decodedException) { - if (PyObjectConversions.TryDecode(valueHandle, typeHandle, typeof(Exception), out object decoded) - && decoded is Exception decodedException) - { - return decodedException; - } - - var raw = new PythonException(type, value, traceback); - raw.Normalize(); - - using var cause = Runtime.PyException_GetCause(raw.Value.Reference); - if (!cause.IsNull() && !cause.IsNone()) - { - using var innerTraceback = Runtime.PyException_GetTraceback(cause); - inner = FromPyErr( - typeHandle: Runtime.PyObject_TYPE(cause), - valueHandle: cause, - tracebackHandle: innerTraceback); - } + return decodedException; } + using var cause = Runtime.PyException_GetCause(valRef); + Exception? inner = FromCause(cause); return new PythonException(type, value, traceback, inner); } - private string GetMessage() => GetMessage(Value, Type); + private static Exception? FromCause(BorrowedReference cause) + { + if (cause == null || cause.IsNone()) return null; - private static string GetMessage(PyObject value, PyType type) + Debug.Assert(Runtime.PyObject_TypeCheck(cause, new BorrowedReference(Exceptions.BaseException))); + + using var innerTraceback = Runtime.PyException_GetTraceback(cause); + return FromPyErr( + typeRef: Runtime.PyObject_TYPE(cause), + valRef: cause, + tbRef: innerTraceback, + out _); + + } + + private static string GetMessage(PyObject? value, PyType type) { using var _ = new Py.GILState(); if (value != null && !value.IsNone()) @@ -208,12 +180,7 @@ private static string GetMessage(PyObject value, PyType type) return value.ToString(); } - if (type != null) - { - return type.Name; - } - - throw new ArgumentException("One of the values must not be null"); + return type.Name; } private static string TracebackToString(PyObject traceback) @@ -238,12 +205,18 @@ private static string TracebackToString(PyObject traceback) /// Restores python error. public void Restore() { - IntPtr gs = PythonEngine.AcquireLock(); + CheckRuntimeIsRunning(); + + using var _ = new Py.GILState(); + + NewReference type = Type.NewReferenceOrNull(); + NewReference value = Value.NewReferenceOrNull(); + NewReference traceback = Traceback.NewReferenceOrNull(); + Runtime.PyErr_Restore( - Type.NewReferenceOrNull().StealNullable(), - Value.NewReferenceOrNull().StealNullable(), - Traceback.NewReferenceOrNull().StealNullable()); - PythonEngine.ReleaseLock(gs); + type: type.Steal(), + val: value.StealNullable(), + tb: traceback.StealNullable()); } /// @@ -255,12 +228,12 @@ public void Restore() /// Returns the exception value as a Python object. /// /// - public PyObject Value { get; private set; } + public PyObject? Value { get; private set; } /// /// Returns the TraceBack as a Python object. /// - public PyObject Traceback { get; } + public PyObject? Traceback { get; } /// /// StackTrace Property @@ -274,35 +247,74 @@ public override string StackTrace { if (Traceback is null) return base.StackTrace; + if (!PythonEngine.IsInitialized && Runtime.Py_IsInitialized() == 0) + return "Python stack unavailable as runtime was shut down\n" + base.StackTrace; + using var _ = new Py.GILState(); return TracebackToString(Traceback) + base.StackTrace; } } - public override string Message => GetMessage(); + public override string Message + { + get + { + if (!PythonEngine.IsInitialized && Runtime.Py_IsInitialized() == 0) + return "Python error message is unavailable as runtime was shut down"; + + return GetMessage(this.Value, this.Type); + } + } + + public bool IsNormalized + { + get + { + if (Value is null) return false; + + CheckRuntimeIsRunning(); + + using var _ = new Py.GILState(); + return Runtime.PyObject_TypeCheck(Value.Reference, Type.Reference); + } + } /// /// Replaces Value with an instance of Type, if Value is not already an instance of Type. /// public void Normalize() { + CheckRuntimeIsRunning(); + IntPtr gs = PythonEngine.AcquireLock(); try { if (Exceptions.ErrorOccurred()) throw new InvalidOperationException("Cannot normalize when an error is set"); + // If an error is set and this PythonException is unnormalized, the error will be cleared and the PythonException will be replaced by a different error. NewReference value = Value.NewReferenceOrNull(); NewReference type = Type.NewReferenceOrNull(); NewReference tb = Traceback.NewReferenceOrNull(); + Runtime.PyErr_NormalizeException(type: ref type, val: ref value, tb: ref tb); + Value = value.MoveToPyObject(); Type = new PyType(type.Steal()); - if (!tb.IsNull()) + try { - int r = Runtime.PyException_SetTraceback(Value.Reference, tb); - ThrowIfIsNotZero(r); + Debug.Assert(Traceback is null == tb.IsNull()); + if (!tb.IsNull()) + { + Debug.Assert(Traceback!.Reference == tb); + + int r = Runtime.PyException_SetTraceback(Value.Reference, tb); + ThrowIfIsNotZero(r); + } + } + finally + { + tb.Dispose(); } - tb.Dispose(); } finally { @@ -316,35 +328,26 @@ public void Normalize() /// public string Format() { - string res; - IntPtr gs = PythonEngine.AcquireLock(); - try - { - var copy = Clone(); - copy.Normalize(); + CheckRuntimeIsRunning(); - if (copy.Traceback != null && copy.Type != null && copy.Value != null) - { - using var traceback = PyModule.Import("traceback"); - var buffer = new StringBuilder(); - using var values = traceback.InvokeMethod("format_exception", copy.Type, copy.Value, copy.Traceback); - foreach (PyObject val in values) - { - buffer.Append(val); - val.Dispose(); - } - res = buffer.ToString(); - } - else - { - res = StackTrace; - } - } - finally + using var _ = new Py.GILState(); + + var copy = Clone(); + copy.Normalize(); + + if (copy.Traceback is null || copy.Value is null) + return StackTrace; + + using var traceback = PyModule.Import("traceback"); + var buffer = new StringBuilder(); + using var values = traceback.InvokeMethod("format_exception", copy.Type, copy.Value, copy.Traceback); + foreach (PyObject val in values) { - PythonEngine.ReleaseLock(gs); + buffer.Append(val); + val.Dispose(); } - return res; + return buffer.ToString(); + } public PythonException Clone() @@ -357,6 +360,12 @@ internal bool Is(IntPtr type) new BorrowedReference(type)) != 0; } + private static void CheckRuntimeIsRunning() + { + if (!PythonEngine.IsInitialized && Runtime.Py_IsInitialized() == 0) + throw new InvalidOperationException("Python runtime must be running"); + } + /// /// Returns true if the current Python exception /// matches the given exception type. diff --git a/src/runtime/pytype.cs b/src/runtime/pytype.cs index 85ee54171..3ce842a4d 100644 --- a/src/runtime/pytype.cs +++ b/src/runtime/pytype.cs @@ -22,6 +22,9 @@ internal PyType(StolenReference reference) : base(EnsureIsType(in reference)) ? null : new PyType(new NewReference(reference).Steal()); + internal static PyType FromReference(BorrowedReference reference) + => FromNullableReference(reference) ?? throw new ArgumentNullException(nameof(reference)); + public string Name { get diff --git a/src/runtime/runtime.cs b/src/runtime/runtime.cs index 194e928c0..e6cf8f13c 100644 --- a/src/runtime/runtime.cs +++ b/src/runtime/runtime.cs @@ -1,8 +1,7 @@ -using System.Reflection.Emit; using System; +using System.Diagnostics; using System.Diagnostics.Contracts; using System.Runtime.InteropServices; -using System.Security; using System.Text; using System.Threading; using System.Collections.Generic; @@ -424,12 +423,8 @@ private static void RunExitFuncs() { atexit = Py.Import("atexit"); } - catch (PythonException e) + catch (PythonException e) when (e.Is(Exceptions.ImportError)) { - if (!e.Is(Exceptions.ImportError)) - { - throw; - } // The runtime may not provided `atexit` module. return; } @@ -586,7 +581,7 @@ public static PyObject None /// internal static void CheckExceptionOccurred() { - if (PyErr_Occurred() != IntPtr.Zero) + if (PyErr_Occurred() != null) { throw PythonException.ThrowLastAsClrException(); } @@ -2094,13 +2089,13 @@ internal static void PyErr_SetString(IntPtr ob, string message) internal static void PyErr_NormalizeException(ref NewReference type, ref NewReference val, ref NewReference tb) => Delegates.PyErr_NormalizeException(ref type, ref val, ref tb); - internal static IntPtr PyErr_Occurred() => Delegates.PyErr_Occurred(); + internal static BorrowedReference PyErr_Occurred() => Delegates.PyErr_Occurred(); internal static void PyErr_Fetch(out NewReference type, out NewReference val, out NewReference tb) => Delegates.PyErr_Fetch(out type, out val, out tb); - internal static void PyErr_Restore(StolenReference ob, StolenReference val, StolenReference tb) => Delegates.PyErr_Restore(ob, val, tb); + internal static void PyErr_Restore(StolenReference type, StolenReference val, StolenReference tb) => Delegates.PyErr_Restore(type, val, tb); internal static void PyErr_Clear() => Delegates.PyErr_Clear(); @@ -2500,7 +2495,7 @@ static Delegates() PyErr_ExceptionMatches = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyErr_ExceptionMatches), GetUnmanagedDll(_PythonDll)); PyErr_GivenExceptionMatches = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyErr_GivenExceptionMatches), GetUnmanagedDll(_PythonDll)); PyErr_NormalizeException = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyErr_NormalizeException), GetUnmanagedDll(_PythonDll)); - PyErr_Occurred = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyErr_Occurred), GetUnmanagedDll(_PythonDll)); + PyErr_Occurred = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyErr_Occurred), GetUnmanagedDll(_PythonDll)); PyErr_Fetch = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyErr_Fetch), GetUnmanagedDll(_PythonDll)); PyErr_Restore = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyErr_Restore), GetUnmanagedDll(_PythonDll)); PyErr_Clear = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyErr_Clear), GetUnmanagedDll(_PythonDll)); @@ -2780,7 +2775,7 @@ static Delegates() internal static delegate* unmanaged[Cdecl] PyErr_ExceptionMatches { get; } internal static delegate* unmanaged[Cdecl] PyErr_GivenExceptionMatches { get; } internal static delegate* unmanaged[Cdecl] PyErr_NormalizeException { get; } - internal static delegate* unmanaged[Cdecl] PyErr_Occurred { get; } + internal static delegate* unmanaged[Cdecl] PyErr_Occurred { get; } internal static delegate* unmanaged[Cdecl] PyErr_Fetch { get; } internal static delegate* unmanaged[Cdecl] PyErr_Restore { get; } internal static delegate* unmanaged[Cdecl] PyErr_Clear { get; } From 146ebf3f3d069046b13a812f649e8303186cbdf5 Mon Sep 17 00:00:00 2001 From: Victor Nova Date: Sat, 10 Apr 2021 20:06:24 -0700 Subject: [PATCH 23/39] fixed type of reference in PyException_SetCause --- src/runtime/exceptions.cs | 6 ++++-- src/runtime/runtime.cs | 6 +++--- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/runtime/exceptions.cs b/src/runtime/exceptions.cs index 669ef9e6f..0540d81ca 100644 --- a/src/runtime/exceptions.cs +++ b/src/runtime/exceptions.cs @@ -295,7 +295,7 @@ public static void SetCause(Exception cause) var currentException = PythonException.FetchCurrentRaw(); currentException.Normalize(); using var causeInstance = Converter.ToPythonReference(cause); - Runtime.PyException_SetCause(currentException.Value.Reference, causeInstance); + Runtime.PyException_SetCause(currentException.Value!.Reference, causeInstance.Steal()); currentException.Restore(); } @@ -392,7 +392,9 @@ internal static IntPtr RaiseTypeError(string message) var typeError = PythonException.FetchCurrentRaw(); typeError.Normalize(); - Runtime.PyException_SetCause(typeError.Value.Reference, cause.Value.Reference); + Runtime.PyException_SetCause( + typeError.Value!.Reference, + new NewReference(cause.Value!.Reference).Steal()); typeError.Restore(); return IntPtr.Zero; diff --git a/src/runtime/runtime.cs b/src/runtime/runtime.cs index e6cf8f13c..9f6b92cac 100644 --- a/src/runtime/runtime.cs +++ b/src/runtime/runtime.cs @@ -2112,7 +2112,7 @@ internal static NewReference PyException_GetTraceback(BorrowedReference ex) /// /// Set the cause associated with the exception to cause. Use NULL to clear it. There is no type check to make sure that cause is either an exception instance or None. This steals a reference to cause. /// - internal static void PyException_SetCause(BorrowedReference ex, BorrowedReference cause) + internal static void PyException_SetCause(BorrowedReference ex, StolenReference cause) => Delegates.PyException_SetCause(ex, cause); internal static int PyException_SetTraceback(BorrowedReference ex, BorrowedReference tb) => Delegates.PyException_SetTraceback(ex, tb); @@ -2516,7 +2516,7 @@ static Delegates() PyDict_GetItemWithError = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyDict_GetItemWithError), GetUnmanagedDll(_PythonDll)); PyException_GetCause = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyException_GetCause), GetUnmanagedDll(_PythonDll)); PyException_GetTraceback = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyException_GetTraceback), GetUnmanagedDll(_PythonDll)); - PyException_SetCause = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyException_SetCause), GetUnmanagedDll(_PythonDll)); + PyException_SetCause = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyException_SetCause), GetUnmanagedDll(_PythonDll)); PyException_SetTraceback = (delegate* unmanaged[Cdecl])GetFunctionByName(nameof(PyException_SetTraceback), GetUnmanagedDll(_PythonDll)); PyThreadState_SetAsyncExcLLP64 = (delegate* unmanaged[Cdecl])GetFunctionByName("PyThreadState_SetAsyncExc", GetUnmanagedDll(_PythonDll)); PyThreadState_SetAsyncExcLP64 = (delegate* unmanaged[Cdecl])GetFunctionByName("PyThreadState_SetAsyncExc", GetUnmanagedDll(_PythonDll)); @@ -2796,7 +2796,7 @@ static Delegates() internal static delegate* unmanaged[Cdecl] PyDict_GetItemWithError { get; } internal static delegate* unmanaged[Cdecl] PyException_GetCause { get; } internal static delegate* unmanaged[Cdecl] PyException_GetTraceback { get; } - internal static delegate* unmanaged[Cdecl] PyException_SetCause { get; } + internal static delegate* unmanaged[Cdecl] PyException_SetCause { get; } internal static delegate* unmanaged[Cdecl] PyException_SetTraceback { get; } internal static delegate* unmanaged[Cdecl] PyThreadState_SetAsyncExcLLP64 { get; } internal static delegate* unmanaged[Cdecl] PyThreadState_SetAsyncExcLP64 { get; } From 272687bfdb97654618701f9eaad770d95993580d Mon Sep 17 00:00:00 2001 From: Victor Nova Date: Sun, 11 Apr 2021 01:01:20 -0700 Subject: [PATCH 24/39] minor fixes to Converter.ToArray: - no longer leaking iterator object on failure - when iteration stops due to error, propagates the error --- src/runtime/converter.cs | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/runtime/converter.cs b/src/runtime/converter.cs index 235b71528..645f31daa 100644 --- a/src/runtime/converter.cs +++ b/src/runtime/converter.cs @@ -888,6 +888,7 @@ private static bool ToArray(IntPtr value, Type obType, out object result, bool s if (!Converter.ToManaged(item, elementType, out obj, setError)) { Runtime.XDecref(item); + Runtime.XDecref(IterObject); return false; } @@ -896,6 +897,12 @@ private static bool ToArray(IntPtr value, Type obType, out object result, bool s } Runtime.XDecref(IterObject); + if (Exceptions.ErrorOccurred()) + { + if (!setError) Exceptions.Clear(); + return false; + } + Array items = Array.CreateInstance(elementType, list.Count); list.CopyTo(items, 0); From 2cd3f6189d09e114ddab7aa1204a1d2a86865373 Mon Sep 17 00:00:00 2001 From: Victor Nova Date: Sun, 11 Apr 2021 01:02:27 -0700 Subject: [PATCH 25/39] added a few debug checks to Exceptions.SetError --- src/runtime/exceptions.cs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/runtime/exceptions.cs b/src/runtime/exceptions.cs index 0540d81ca..6a3f2ec6c 100644 --- a/src/runtime/exceptions.cs +++ b/src/runtime/exceptions.cs @@ -1,8 +1,8 @@ using System; +using System.Diagnostics; using System.Reflection; using System.Runtime.ExceptionServices; using System.Runtime.InteropServices; -using System.Text; namespace Python.Runtime { @@ -260,6 +260,8 @@ public static void SetError(IntPtr type, IntPtr exceptionObject) /// public static bool SetError(Exception e) { + Debug.Assert(e is not null); + // Because delegates allow arbitrary nesting of Python calling // managed calling Python calling... etc. it is possible that we // might get a managed exception raised that is a wrapper for a @@ -281,6 +283,8 @@ public static bool SetError(Exception e) if (Runtime.PyObject_SetAttrString(instance, DispatchInfoAttribute, pyInfo) != 0) return false; + Debug.Assert(Runtime.PyObject_TypeCheck(instance, new BorrowedReference(BaseException))); + var type = Runtime.PyObject_TYPE(instance); Runtime.PyErr_SetObject(type, instance); return true; From e79f041f3a72e1855e8b0239ccadf954f58d9fec Mon Sep 17 00:00:00 2001 From: Victor Nova Date: Sun, 11 Apr 2021 01:03:38 -0700 Subject: [PATCH 26/39] method binding failure now supports non-Python exception causes --- src/runtime/methodbinder.cs | 8 ++++---- src/runtime/pythonexception.cs | 4 ++++ 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/runtime/methodbinder.cs b/src/runtime/methodbinder.cs index 6b0976b97..362b1a19c 100644 --- a/src/runtime/methodbinder.cs +++ b/src/runtime/methodbinder.cs @@ -341,13 +341,13 @@ public MatchedMethod(int kwargsMatched, int defaultsNeeded, object[] margs, int private readonly struct MismatchedMethod { - public MismatchedMethod(PythonException exception, MethodBase mb) + public MismatchedMethod(Exception exception, MethodBase mb) { Exception = exception; Method = mb; } - public PythonException Exception { get; } + public Exception Exception { get; } public MethodBase Method { get; } } @@ -438,8 +438,8 @@ internal Binding Bind(IntPtr inst, IntPtr args, IntPtr kw, MethodBase info, Meth outs: out outs); if (margs == null) { - mismatchedMethods.Add(new MismatchedMethod(PythonException.FetchCurrentRaw(), mi)); - Exceptions.Clear(); + var mismatchCause = PythonException.FetchCurrent(); + mismatchedMethods.Add(new MismatchedMethod(mismatchCause, mi)); continue; } if (isOperator) diff --git a/src/runtime/pythonexception.cs b/src/runtime/pythonexception.cs index 3f4bd8154..4e594242c 100644 --- a/src/runtime/pythonexception.cs +++ b/src/runtime/pythonexception.cs @@ -94,6 +94,10 @@ internal static PythonException FetchCurrentRaw() } } + internal static Exception FetchCurrent() + => FetchCurrentOrNull(out _) + ?? throw new InvalidOperationException("No exception is set"); + private static ExceptionDispatchInfo? TryGetDispatchInfo(BorrowedReference exception) { if (exception.IsNull) return null; From d068f365b93903858cddd928a143b20564e0c00a Mon Sep 17 00:00:00 2001 From: Victor Nova Date: Sun, 11 Apr 2021 01:05:26 -0700 Subject: [PATCH 27/39] XDecref now checks, that refcount is positive in debug builds --- src/runtime/runtime.cs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/runtime/runtime.cs b/src/runtime/runtime.cs index 9f6b92cac..3929494d7 100644 --- a/src/runtime/runtime.cs +++ b/src/runtime/runtime.cs @@ -714,6 +714,9 @@ internal static IntPtr SelfIncRef(IntPtr op) internal static unsafe void XDecref(IntPtr op) { +#if DEBUG + Debug.Assert(op == IntPtr.Zero || Refcount(op) > 0); +#endif #if !CUSTOM_INCDEC_REF Py_DecRef(op); return; From 4877fe7d3f52c1dd1cdfdc7e79b263317ee57c13 Mon Sep 17 00:00:00 2001 From: Victor Nova Date: Sun, 11 Apr 2021 03:09:30 -0700 Subject: [PATCH 28/39] fixed __cause__ on overload bind failure and array conversion --- src/runtime/converter.cs | 5 +++++ src/runtime/methodbinder.cs | 2 ++ 2 files changed, 7 insertions(+) diff --git a/src/runtime/converter.cs b/src/runtime/converter.cs index 645f31daa..1425c03e1 100644 --- a/src/runtime/converter.cs +++ b/src/runtime/converter.cs @@ -817,9 +817,14 @@ private static bool ToPrimitive(IntPtr value, Type obType, out object result, bo private static void SetConversionError(IntPtr value, Type target) { + // PyObject_Repr might clear the error + Runtime.PyErr_Fetch(out var causeType, out var causeVal, out var causeTrace); + IntPtr ob = Runtime.PyObject_Repr(value); string src = Runtime.GetManagedString(ob); Runtime.XDecref(ob); + + Runtime.PyErr_Restore(causeType.StealNullable(), causeVal.StealNullable(), causeTrace.StealNullable()); Exceptions.RaiseTypeError($"Cannot convert {src} to {target}"); } diff --git a/src/runtime/methodbinder.cs b/src/runtime/methodbinder.cs index 362b1a19c..b2f844900 100644 --- a/src/runtime/methodbinder.cs +++ b/src/runtime/methodbinder.cs @@ -926,7 +926,9 @@ internal virtual IntPtr Invoke(IntPtr inst, IntPtr args, IntPtr kw, MethodBase i } value.Append(": "); + Runtime.PyErr_Fetch(out var errType, out var errVal, out var errTrace); AppendArgumentTypes(to: value, args); + Runtime.PyErr_Restore(errType.StealNullable(), errVal.StealNullable(), errTrace.StealNullable()); Exceptions.RaiseTypeError(value.ToString()); return IntPtr.Zero; } From ed594c1cf4920428671f128b81498df3b24907b3 Mon Sep 17 00:00:00 2001 From: Victor Nova Date: Sun, 11 Apr 2021 12:35:37 -0700 Subject: [PATCH 29/39] cache PythonException message --- src/runtime/pythonexception.cs | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/src/runtime/pythonexception.cs b/src/runtime/pythonexception.cs index 4e594242c..13316aef9 100644 --- a/src/runtime/pythonexception.cs +++ b/src/runtime/pythonexception.cs @@ -14,16 +14,19 @@ namespace Python.Runtime /// public class PythonException : System.Exception { - public PythonException(PyType type, PyObject? value, PyObject? traceback, - Exception? innerException) - : base("An exception has occurred in Python code. See Message property for details.", innerException) + string message, Exception? innerException) + : base(message, innerException) { Type = type ?? throw new ArgumentNullException(nameof(type)); Value = value; Traceback = traceback; } + public PythonException(PyType type, PyObject? value, PyObject? traceback, + Exception? innerException) + : this(type, value, traceback, GetMessage(value, type), innerException) { } + public PythonException(PyType type, PyObject? value, PyObject? traceback) : this(type, value, traceback, innerException: null) { } @@ -178,7 +181,8 @@ private static Exception FromPyErr(BorrowedReference typeRef, BorrowedReference private static string GetMessage(PyObject? value, PyType type) { - using var _ = new Py.GILState(); + if (type is null) throw new ArgumentNullException(nameof(type)); + if (value != null && !value.IsNone()) { return value.ToString(); @@ -259,17 +263,6 @@ public override string StackTrace } } - public override string Message - { - get - { - if (!PythonEngine.IsInitialized && Runtime.Py_IsInitialized() == 0) - return "Python error message is unavailable as runtime was shut down"; - - return GetMessage(this.Value, this.Type); - } - } - public bool IsNormalized { get @@ -355,7 +348,8 @@ public string Format() } public PythonException Clone() - => new PythonException(Type, Value, Traceback, InnerException); + => new PythonException(type: Type, value: Value, traceback: Traceback, + Message, InnerException); internal bool Is(IntPtr type) { From e5bce06b4819abfd77542215362a5c26cdb9b26e Mon Sep 17 00:00:00 2001 From: Victor Nova Date: Sun, 11 Apr 2021 12:37:27 -0700 Subject: [PATCH 30/39] minor code cleanup --- src/runtime/clrobject.cs | 5 ++++- src/runtime/exceptions.cs | 25 ++++++++----------------- 2 files changed, 12 insertions(+), 18 deletions(-) diff --git a/src/runtime/clrobject.cs b/src/runtime/clrobject.cs index 0aa829ee6..2d402596a 100644 --- a/src/runtime/clrobject.cs +++ b/src/runtime/clrobject.cs @@ -33,7 +33,7 @@ internal CLRObject(object ob, IntPtr tp) // Fix the BaseException args (and __cause__ in case of Python 3) // slot if wrapping a CLR exception - Exceptions.SetArgsAndCause(py); + Exceptions.SetArgsAndCause(ObjectReference); } protected CLRObject() @@ -78,6 +78,9 @@ internal static IntPtr GetInstHandle(object ob) return co.pyHandle; } + internal static NewReference GetReference(object ob) + => NewReference.DangerousFromPointer(GetInstHandle(ob)); + internal static CLRObject Restore(object ob, IntPtr pyHandle, InterDomainContext context) { CLRObject co = new CLRObject() diff --git a/src/runtime/exceptions.cs b/src/runtime/exceptions.cs index 6a3f2ec6c..02a320c71 100644 --- a/src/runtime/exceptions.cs +++ b/src/runtime/exceptions.cs @@ -24,19 +24,10 @@ internal ExceptionClassObject(Type tp) : base(tp) { } - internal static Exception ToException(IntPtr ob) + internal static Exception ToException(BorrowedReference ob) { var co = GetManagedObject(ob) as CLRObject; - if (co == null) - { - return null; - } - var e = co.inst as Exception; - if (e == null) - { - return null; - } - return e; + return co?.inst as Exception; } /// @@ -44,7 +35,7 @@ internal static Exception ToException(IntPtr ob) /// public new static IntPtr tp_repr(IntPtr ob) { - Exception e = ToException(ob); + Exception e = ToException(new BorrowedReference(ob)); if (e == null) { return Exceptions.RaiseTypeError("invalid object"); @@ -67,7 +58,7 @@ internal static Exception ToException(IntPtr ob) /// public new static IntPtr tp_str(IntPtr ob) { - Exception e = ToException(ob); + Exception e = ToException(new BorrowedReference(ob)); if (e == null) { return Exceptions.RaiseTypeError("invalid object"); @@ -157,7 +148,7 @@ internal static void Shutdown() /// pointer. /// /// The python object wrapping - internal static void SetArgsAndCause(IntPtr ob) + internal static void SetArgsAndCause(BorrowedReference ob) { // e: A CLR Exception Exception e = ExceptionClassObject.ToException(ob); @@ -178,13 +169,13 @@ internal static void SetArgsAndCause(IntPtr ob) args = Runtime.PyTuple_New(0); } - Marshal.WriteIntPtr(ob, ExceptionOffset.args, args); + Marshal.WriteIntPtr(ob.DangerousGetAddress(), ExceptionOffset.args, args); if (e.InnerException != null) { // Note: For an AggregateException, InnerException is only the first of the InnerExceptions. - IntPtr cause = CLRObject.GetInstHandle(e.InnerException); - Marshal.WriteIntPtr(ob, ExceptionOffset.cause, cause); + using var cause = CLRObject.GetReference(e.InnerException); + Runtime.PyException_SetCause(ob, cause.Steal()); } } From 6819e7b5ea8881c048450079630d79e28261782a Mon Sep 17 00:00:00 2001 From: Victor Nova Date: Sun, 11 Apr 2021 12:57:00 -0700 Subject: [PATCH 31/39] improved handling of dict offset in object instances --- src/runtime/interop.cs | 5 ++++- src/runtime/managedtype.cs | 8 ++++++-- src/runtime/typemanager.cs | 2 +- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/runtime/interop.cs b/src/runtime/interop.cs index c5958e0f7..cf6345b30 100644 --- a/src/runtime/interop.cs +++ b/src/runtime/interop.cs @@ -185,7 +185,10 @@ public static int magic(IntPtr type) public static int TypeDictOffset(IntPtr type) { - return ManagedDataOffsets.DictOffset(type); + Debug.Assert(TypeOffset.tp_dictoffset > 0); + int dictOffset = Marshal.ReadInt32(type, TypeOffset.tp_dictoffset); + Debug.Assert(dictOffset > 0); + return dictOffset; } public static int Size(IntPtr pyType) diff --git a/src/runtime/managedtype.cs b/src/runtime/managedtype.cs index a534e5d99..3cd920ea9 100644 --- a/src/runtime/managedtype.cs +++ b/src/runtime/managedtype.cs @@ -260,13 +260,17 @@ protected static void ClearObjectDict(IntPtr ob) protected static IntPtr GetObjectDict(IntPtr ob) { IntPtr type = Runtime.PyObject_TYPE(ob); - return Marshal.ReadIntPtr(ob, ObjectOffset.TypeDictOffset(type)); + int dictOffset = ObjectOffset.TypeDictOffset(type); + if (dictOffset == 0) return IntPtr.Zero; + return Marshal.ReadIntPtr(ob, dictOffset); } protected static void SetObjectDict(IntPtr ob, IntPtr value) { IntPtr type = Runtime.PyObject_TYPE(ob); - Marshal.WriteIntPtr(ob, ObjectOffset.TypeDictOffset(type), value); + int dictOffset = ObjectOffset.TypeDictOffset(type); + Debug.Assert(dictOffset > 0); + Marshal.WriteIntPtr(ob, dictOffset, value); } } } diff --git a/src/runtime/typemanager.cs b/src/runtime/typemanager.cs index fde8fb719..c899b60e2 100644 --- a/src/runtime/typemanager.cs +++ b/src/runtime/typemanager.cs @@ -165,7 +165,7 @@ internal static IntPtr CreateType(Type impl) // Set tp_basicsize to the size of our managed instance objects. Marshal.WriteIntPtr(type, TypeOffset.tp_basicsize, (IntPtr)ob_size); - var offset = (IntPtr)ObjectOffset.TypeDictOffset(type); + var offset = (IntPtr)ManagedDataOffsets.DictOffset(type); Marshal.WriteIntPtr(type, TypeOffset.tp_dictoffset, offset); SlotsHolder slotsHolder = CreateSolotsHolder(type); From 6679d1ce5924b93162cd44c8a7b568ea999f89a1 Mon Sep 17 00:00:00 2001 From: Victor Nova Date: Sun, 11 Apr 2021 12:58:24 -0700 Subject: [PATCH 32/39] added a few debug checks --- src/runtime/runtime.cs | 22 ++++++++++++++++++++-- 1 file changed, 20 insertions(+), 2 deletions(-) diff --git a/src/runtime/runtime.cs b/src/runtime/runtime.cs index 3929494d7..383de3c30 100644 --- a/src/runtime/runtime.cs +++ b/src/runtime/runtime.cs @@ -1128,13 +1128,31 @@ internal static int PyObject_Compare(IntPtr value1, IntPtr value2) internal static nint PyObject_Hash(IntPtr op) => Delegates.PyObject_Hash(op); - internal static IntPtr PyObject_Repr(IntPtr pointer) => Delegates.PyObject_Repr(pointer); + internal static IntPtr PyObject_Repr(IntPtr pointer) + { + AssertNoErorSet(); + return Delegates.PyObject_Repr(pointer); + } internal static IntPtr PyObject_Str(IntPtr pointer) => Delegates.PyObject_Str(pointer); - internal static IntPtr PyObject_Unicode(IntPtr pointer) => Delegates.PyObject_Unicode(pointer); + internal static IntPtr PyObject_Unicode(IntPtr pointer) + { + AssertNoErorSet(); + + return Delegates.PyObject_Unicode(pointer); + } + + [Conditional("DEBUG")] + internal static void AssertNoErorSet() + { + if (Exceptions.ErrorOccurred()) + throw new InvalidOperationException( + "Can't call with exception set", + PythonException.FetchCurrent()); + } internal static IntPtr PyObject_Dir(IntPtr pointer) => Delegates.PyObject_Dir(pointer); From 2e57b0419bb49fb95165c1641b36c4e389af037b Mon Sep 17 00:00:00 2001 From: Victor Nova Date: Sat, 22 May 2021 18:09:39 -0700 Subject: [PATCH 33/39] + class diagram for ManagedType and subclasses --- src/runtime/ManagedTypes.cd | 165 ++++++++++++++++++++++++++++++++++++ 1 file changed, 165 insertions(+) create mode 100644 src/runtime/ManagedTypes.cd diff --git a/src/runtime/ManagedTypes.cd b/src/runtime/ManagedTypes.cd new file mode 100644 index 000000000..9c0f73b4f --- /dev/null +++ b/src/runtime/ManagedTypes.cd @@ -0,0 +1,165 @@ + + + + + + FAAAAgAIAAAEDAAAAAAAAEACIACJAAIAAAAAAAIAAAQ= + classbase.cs + + + + + + AAAAAABIAAABDAAAIAIAAAAAAAAAAAAAAAAACAiAAAQ= + classderived.cs + + + + + + AAAAAAAAABAAAAAAAAAAACAAIAAJAAAAIAAAAACAAAI= + arrayobject.cs + + + + + + AAABAAAIAAAAAAAAIAAAAAAAAAAAAIAAACAAAACAAAA= + classobject.cs + + + + + + AAAACAAAAAAABABAAAAACAAAABAJAAAAAAAAAAIAAAQ= + constructorbinding.cs + + + + + + EAAAAAAAAAAAAAAAAAACAAACBIAAAAJAAAAAAAAAAAA= + clrobject.cs + + + + + + AAAAAEAgIAQABAAAAABAAAAAIAIAAAAAAhAQAAAAKBA= + moduleobject.cs + + + + + + AAAACAAAAAAABAAAAAAACAAAABAJAAAAAAAAAAIAEAQ= + constructorbinding.cs + + + + + + AAABAAAAAAAAAABAAAAAAEAAIAACAAAAAAAAAACAAAA= + delegateobject.cs + + + + + + AAAAAAAAAAAADAAAIAAAEABAAAAAAAACAAAAAAIAAAQ= + eventbinding.cs + + + + + + AAACAAAAAAAAAAAAAAIAAIAAAAAEAAAAQABAAAIBEAQ= + eventobject.cs + + + + + + AAAAAgAAAAAAACAAAAAAAAAAAAAAAAAAAAAAAAIAAAA= + exceptions.cs + + + + + + AAAAAAAAAAAAAAAAAAAAAAECAAAAAEEBAAAAAAABAAQ= + extensiontype.cs + + + + + + AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAQAAAAAIBEAA= + fieldobject.cs + + + + + + AAAAAAAAAAAAAAAAAAAAAAAggAAAAAAAEAACAACAAAA= + interfaceobject.cs + + + + + + UCBBgoBAIUgAAAEAACAAsAACAgAIABIAQYAAACIYIBA= + managedtype.cs + + + + + + AQAAAAAICBAAAQBAAAABAAIAAgABAAABAAAAUBCAAAQ= + metatype.cs + + + + + + EAAAAAAAAIAADABAIAAAAAAAAAgBAAAAUgAAAAIAAAQ= + methodbinding.cs + + + + + + FIADAAAAAAAIBAAAIAAIAAAIAAgFAAAAUAAgAAIAEAQ= + methodobject.cs + + + + + + AAAAAAAAAAAAAABAAAAAAAAAAAAAAAAAAAAAAAIAAAA= + modulefunctionobject.cs + + + + + + ECCCCkAAAAAABAAAAAABAAACAAAIAIIAEAAAAAIACAQ= + moduleobject.cs + + + + + + AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA= + modulepropertyobject.cs + + + + + + AAAAAAAAAAAAAAAAAAAAAACAAAAAAAAAQAAAAAIBEAg= + propertyobject.cs + + + + \ No newline at end of file From 539ce815de51a935805b7a7baad8757d8004b660 Mon Sep 17 00:00:00 2001 From: Victor Nova Date: Sat, 22 May 2021 20:08:25 -0700 Subject: [PATCH 34/39] added OverloadMapper to ManagedTypes class diagram --- src/runtime/ManagedTypes.cd | 53 +++++++++++++++++++++++++++++-------- 1 file changed, 42 insertions(+), 11 deletions(-) diff --git a/src/runtime/ManagedTypes.cd b/src/runtime/ManagedTypes.cd index 9c0f73b4f..385ae7117 100644 --- a/src/runtime/ManagedTypes.cd +++ b/src/runtime/ManagedTypes.cd @@ -29,9 +29,9 @@ - + - AAAACAAAAAAABABAAAAACAAAABAJAAAAAAAAAAIAAAQ= + AAAACAAAAAAABABAAAAACAAAABAJAEAAAAAAAAIAAAA= constructorbinding.cs @@ -52,7 +52,7 @@ - AAAACAAAAAAABAAAAAAACAAAABAJAAAAAAAAAAIAEAQ= + AAAACAAAAAAABAAAAAAACAAAABAJAEAAAAAAAAIAEAA= constructorbinding.cs @@ -64,7 +64,15 @@ - + + + + + + + + + AAAAAAAAAAAADAAAIAAAEABAAAAAAAACAAAAAAIAAAQ= eventbinding.cs @@ -87,12 +95,12 @@ - AAAAAAAAAAAAAAAAAAAAAAECAAAAAEEBAAAAAAABAAQ= + AAAAAAAAAAAAAAAAAAAAAAACAAAAAEEBAAAAAAABAAQ= extensiontype.cs - + AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAQAAAAAIBEAA= fieldobject.cs @@ -120,21 +128,29 @@ - + + + + + + + + + EAAAAAAAAIAADABAIAAAAAAAAAgBAAAAUgAAAAIAAAQ= methodbinding.cs - + FIADAAAAAAAIBAAAIAAIAAAIAAgFAAAAUAAgAAIAEAQ= methodobject.cs - + AAAAAAAAAAAAAABAAAAAAAAAAAAAAAAAAAAAAAIAAAA= modulefunctionobject.cs @@ -148,18 +164,33 @@ - + AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA= modulepropertyobject.cs - + AAAAAAAAAAAAAAAAAAAAAACAAAAAAAAAQAAAAAIBEAg= propertyobject.cs + + + + + + + + + + + + AAAAAAAAAAAAAAAAIAAAAAAAAAABAAAAAgAAAAIAAAQ= + overload.cs + + \ No newline at end of file From 25e38640c6367c04b644c94b702a5ef2f2e13248 Mon Sep 17 00:00:00 2001 From: Victor Nova Date: Sat, 22 May 2021 20:10:19 -0700 Subject: [PATCH 35/39] refactored tp_dealloc in ExtensionType and descendants --- src/runtime/constructorbinding.cs | 20 ++++++------------- src/runtime/eventbinding.cs | 11 +++-------- src/runtime/eventobject.cs | 13 +++++-------- src/runtime/extensiontype.cs | 18 +++++------------ src/runtime/methodbinding.cs | 24 ++++++++++------------- src/runtime/methodobject.cs | 32 ++++++++++++++----------------- src/runtime/moduleobject.cs | 13 ++++++------- src/runtime/native/TypeOffset.cs | 1 - src/runtime/overload.cs | 10 +++------- 9 files changed, 52 insertions(+), 90 deletions(-) diff --git a/src/runtime/constructorbinding.cs b/src/runtime/constructorbinding.cs index 6706c2b48..ab4a7af0a 100644 --- a/src/runtime/constructorbinding.cs +++ b/src/runtime/constructorbinding.cs @@ -149,14 +149,10 @@ public static IntPtr tp_repr(IntPtr ob) return self.repr; } - /// - /// ConstructorBinding dealloc implementation. - /// - public new static void tp_dealloc(IntPtr ob) + protected override void Dealloc() { - var self = (ConstructorBinding)GetManagedObject(ob); - Runtime.XDecref(self.repr); - self.Dealloc(); + Runtime.Py_CLEAR(ref this.repr); + base.Dealloc(); } public static int tp_clear(IntPtr ob) @@ -252,14 +248,10 @@ public static IntPtr tp_repr(IntPtr ob) return self.repr; } - /// - /// ConstructorBinding dealloc implementation. - /// - public new static void tp_dealloc(IntPtr ob) + protected override void Dealloc() { - var self = (BoundContructor)GetManagedObject(ob); - Runtime.XDecref(self.repr); - self.Dealloc(); + Runtime.Py_CLEAR(ref this.repr); + base.Dealloc(); } public static int tp_clear(IntPtr ob) diff --git a/src/runtime/eventbinding.cs b/src/runtime/eventbinding.cs index 3f5b7b007..60b9bba92 100644 --- a/src/runtime/eventbinding.cs +++ b/src/runtime/eventbinding.cs @@ -103,15 +103,10 @@ public static IntPtr tp_repr(IntPtr ob) return Runtime.PyString_FromString(s); } - - /// - /// EventBinding dealloc implementation. - /// - public new static void tp_dealloc(IntPtr ob) + protected override void Dealloc() { - var self = (EventBinding)GetManagedObject(ob); - Runtime.XDecref(self.target); - self.Dealloc(); + Runtime.Py_CLEAR(ref this.target); + base.Dealloc(); } public static int tp_clear(IntPtr ob) diff --git a/src/runtime/eventobject.cs b/src/runtime/eventobject.cs index 4dc785ddd..e9bd98821 100644 --- a/src/runtime/eventobject.cs +++ b/src/runtime/eventobject.cs @@ -198,17 +198,14 @@ public static IntPtr tp_repr(IntPtr ob) } - /// - /// Descriptor dealloc implementation. - /// - public new static void tp_dealloc(IntPtr ob) + protected override void Dealloc() { - var self = (EventObject)GetManagedObject(ob); - if (self.unbound != null) + if (this.unbound is not null) { - Runtime.XDecref(self.unbound.pyHandle); + Runtime.XDecref(this.unbound.pyHandle); + this.unbound = null; } - self.Dealloc(); + base.Dealloc(); } } diff --git a/src/runtime/extensiontype.cs b/src/runtime/extensiontype.cs index 34a82fe37..db9eb0f72 100644 --- a/src/runtime/extensiontype.cs +++ b/src/runtime/extensiontype.cs @@ -54,20 +54,12 @@ void SetupGc () } - /// - /// Common finalization code to support custom tp_deallocs. - /// - public static void FinalizeObject(ManagedType self) + protected virtual void Dealloc() { - ClearObjectDict(self.pyHandle); - Runtime.PyObject_GC_Del(self.pyHandle); + ClearObjectDict(this.pyHandle); + Runtime.PyObject_GC_Del(this.pyHandle); // Not necessary for decref of `tpHandle`. - self.FreeGCHandle(); - } - - protected void Dealloc() - { - FinalizeObject(this); + this.FreeGCHandle(); } /// @@ -104,7 +96,7 @@ public static void tp_dealloc(IntPtr ob) // Clean up a Python instance of this extension type. This // frees the allocated Python object and decrefs the type. var self = (ExtensionType)GetManagedObject(ob); - self.Dealloc(); + self?.Dealloc(); } protected override void OnLoad(InterDomainContext context) diff --git a/src/runtime/methodbinding.cs b/src/runtime/methodbinding.cs index f33015ba4..783717189 100644 --- a/src/runtime/methodbinding.cs +++ b/src/runtime/methodbinding.cs @@ -31,7 +31,7 @@ public MethodBinding(MethodObject m, IntPtr target, IntPtr targetType) { Runtime.XIncref(targetType); } - + this.targetType = targetType; this.info = null; @@ -42,12 +42,6 @@ public MethodBinding(MethodObject m, IntPtr target) : this(m, target, IntPtr.Zer { } - private void ClearMembers() - { - Runtime.Py_CLEAR(ref target); - Runtime.Py_CLEAR(ref targetType); - } - /// /// Implement binding of generic methods using the subscript syntax []. /// @@ -235,14 +229,16 @@ public static IntPtr tp_repr(IntPtr ob) return Runtime.PyString_FromString($"<{type} method '{name}'>"); } - /// - /// MethodBinding dealloc implementation. - /// - public new static void tp_dealloc(IntPtr ob) + private void ClearMembers() { - var self = (MethodBinding)GetManagedObject(ob); - self.ClearMembers(); - self.Dealloc(); + Runtime.Py_CLEAR(ref target); + Runtime.Py_CLEAR(ref targetType); + } + + protected override void Dealloc() + { + this.ClearMembers(); + base.Dealloc(); } public static int tp_clear(IntPtr ob) diff --git a/src/runtime/methodobject.cs b/src/runtime/methodobject.cs index 37c01f5c5..5fa965f1b 100644 --- a/src/runtime/methodobject.cs +++ b/src/runtime/methodobject.cs @@ -120,16 +120,6 @@ internal bool IsStatic() return is_static; } - private void ClearMembers() - { - Runtime.Py_CLEAR(ref doc); - if (unbound != null) - { - Runtime.XDecref(unbound.pyHandle); - unbound = null; - } - } - /// /// Descriptor __getattribute__ implementation. /// @@ -210,15 +200,21 @@ public static IntPtr tp_repr(IntPtr ob) return Runtime.PyString_FromString($""); } - /// - /// Descriptor dealloc implementation. - /// - public new static void tp_dealloc(IntPtr ob) + private void ClearMembers() { - var self = (MethodObject)GetManagedObject(ob); - self.ClearMembers(); - ClearObjectDict(ob); - self.Dealloc(); + Runtime.Py_CLEAR(ref doc); + if (unbound != null) + { + Runtime.XDecref(unbound.pyHandle); + unbound = null; + } + } + + protected override void Dealloc() + { + this.ClearMembers(); + ClearObjectDict(this.pyHandle); + base.Dealloc(); } public static int tp_clear(IntPtr ob) diff --git a/src/runtime/moduleobject.cs b/src/runtime/moduleobject.cs index 8655253df..8892390c3 100644 --- a/src/runtime/moduleobject.cs +++ b/src/runtime/moduleobject.cs @@ -274,7 +274,7 @@ public static IntPtr tp_getattro(IntPtr ob, IntPtr key) Exceptions.SetError(e); return IntPtr.Zero; } - + if (attr == null) { @@ -295,11 +295,10 @@ public static IntPtr tp_repr(IntPtr ob) return Runtime.PyString_FromString($""); } - public new static void tp_dealloc(IntPtr ob) + protected override void Dealloc() { - var self = (ModuleObject)GetManagedObject(ob); - tp_clear(ob); - self.Dealloc(); + tp_clear(this.pyHandle); + base.Dealloc(); } public static int tp_traverse(IntPtr ob, IntPtr visit, IntPtr arg) @@ -345,7 +344,7 @@ protected override void OnSave(InterDomainContext context) if ((Runtime.PyDict_DelItemString(DictRef, pair.Key) == -1) && (Exceptions.ExceptionMatches(Exceptions.KeyError))) { - // Trying to remove a key that's not in the dictionary + // Trying to remove a key that's not in the dictionary // raises an error. We don't care about it. Runtime.PyErr_Clear(); } @@ -496,7 +495,7 @@ public static Assembly AddReference(string name) /// clr.GetClrType(IComparable) gives you the Type for IComparable, /// that you can e.g. perform reflection on. Similar to typeof(IComparable) in C# /// or clr.GetClrType(IComparable) in IronPython. - /// + /// /// /// /// The Type object diff --git a/src/runtime/native/TypeOffset.cs b/src/runtime/native/TypeOffset.cs index 4e5a726bc..a73a9ae43 100644 --- a/src/runtime/native/TypeOffset.cs +++ b/src/runtime/native/TypeOffset.cs @@ -153,7 +153,6 @@ static void ValidateRequiredOffsetsPresent(PropertyInfo[] offsetProperties) "__instancecheck__", "__subclasscheck__", "AddReference", - "FinalizeObject", "FindAssembly", "get_SuppressDocs", "get_SuppressOverloads", diff --git a/src/runtime/overload.cs b/src/runtime/overload.cs index e9fa91d3b..48fabca4a 100644 --- a/src/runtime/overload.cs +++ b/src/runtime/overload.cs @@ -58,14 +58,10 @@ public static IntPtr tp_repr(IntPtr op) return doc; } - /// - /// OverloadMapper dealloc implementation. - /// - public new static void tp_dealloc(IntPtr ob) + protected override void Dealloc() { - var self = (OverloadMapper)GetManagedObject(ob); - Runtime.XDecref(self.target); - self.Dealloc(); + Runtime.Py_CLEAR(ref this.target); + base.Dealloc(); } } } From 5bca333897bcb546a1fa079daf6f55559ca0a8f3 Mon Sep 17 00:00:00 2001 From: Victor Nova Date: Sat, 22 May 2021 22:39:57 -0700 Subject: [PATCH 36/39] refactored tp_clear in ExtensionType and descendants into a virtual C# function Clear --- src/runtime/constructorbinding.cs | 14 ++++++-------- src/runtime/eventbinding.cs | 7 +++---- src/runtime/eventobject.cs | 10 ++++++++++ src/runtime/extensiontype.cs | 13 ++++++++++--- src/runtime/methodbinding.cs | 7 +++---- src/runtime/methodobject.cs | 9 ++++----- src/runtime/moduleobject.cs | 25 ++++++++++++------------- src/runtime/overload.cs | 6 ++++++ 8 files changed, 54 insertions(+), 37 deletions(-) diff --git a/src/runtime/constructorbinding.cs b/src/runtime/constructorbinding.cs index ab4a7af0a..da35aafd1 100644 --- a/src/runtime/constructorbinding.cs +++ b/src/runtime/constructorbinding.cs @@ -155,11 +155,10 @@ protected override void Dealloc() base.Dealloc(); } - public static int tp_clear(IntPtr ob) + protected override void Clear() { - var self = (ConstructorBinding)GetManagedObject(ob); - Runtime.Py_CLEAR(ref self.repr); - return 0; + Runtime.Py_CLEAR(ref this.repr); + base.Clear(); } public static int tp_traverse(IntPtr ob, IntPtr visit, IntPtr arg) @@ -254,11 +253,10 @@ protected override void Dealloc() base.Dealloc(); } - public static int tp_clear(IntPtr ob) + protected override void Clear() { - var self = (BoundContructor)GetManagedObject(ob); - Runtime.Py_CLEAR(ref self.repr); - return 0; + Runtime.Py_CLEAR(ref this.repr); + base.Clear(); } public static int tp_traverse(IntPtr ob, IntPtr visit, IntPtr arg) diff --git a/src/runtime/eventbinding.cs b/src/runtime/eventbinding.cs index 60b9bba92..8fb0ce250 100644 --- a/src/runtime/eventbinding.cs +++ b/src/runtime/eventbinding.cs @@ -109,11 +109,10 @@ protected override void Dealloc() base.Dealloc(); } - public static int tp_clear(IntPtr ob) + protected override void Clear() { - var self = (EventBinding)GetManagedObject(ob); - Runtime.Py_CLEAR(ref self.target); - return 0; + Runtime.Py_CLEAR(ref this.target); + base.Clear(); } } } diff --git a/src/runtime/eventobject.cs b/src/runtime/eventobject.cs index e9bd98821..a0f06c375 100644 --- a/src/runtime/eventobject.cs +++ b/src/runtime/eventobject.cs @@ -207,6 +207,16 @@ protected override void Dealloc() } base.Dealloc(); } + + protected override void Clear() + { + if (this.unbound is not null) + { + Runtime.XDecref(this.unbound.pyHandle); + this.unbound = null; + } + base.Clear(); + } } diff --git a/src/runtime/extensiontype.cs b/src/runtime/extensiontype.cs index db9eb0f72..38fe238de 100644 --- a/src/runtime/extensiontype.cs +++ b/src/runtime/extensiontype.cs @@ -62,6 +62,9 @@ protected virtual void Dealloc() this.FreeGCHandle(); } + /// DecRefs and nulls any fields pointing back to Python + protected virtual void Clear() { } + /// /// Type __setattr__ implementation. /// @@ -88,9 +91,6 @@ public static int tp_descr_set(IntPtr ds, IntPtr ob, IntPtr val) } - /// - /// Default dealloc implementation. - /// public static void tp_dealloc(IntPtr ob) { // Clean up a Python instance of this extension type. This @@ -99,6 +99,13 @@ public static void tp_dealloc(IntPtr ob) self?.Dealloc(); } + public static int tp_clear(IntPtr ob) + { + var self = (ExtensionType)GetManagedObject(ob); + self?.Clear(); + return 0; + } + protected override void OnLoad(InterDomainContext context) { base.OnLoad(context); diff --git a/src/runtime/methodbinding.cs b/src/runtime/methodbinding.cs index 783717189..a842dd308 100644 --- a/src/runtime/methodbinding.cs +++ b/src/runtime/methodbinding.cs @@ -241,11 +241,10 @@ protected override void Dealloc() base.Dealloc(); } - public static int tp_clear(IntPtr ob) + protected override void Clear() { - var self = (MethodBinding)GetManagedObject(ob); - self.ClearMembers(); - return 0; + this.ClearMembers(); + base.Clear(); } protected override void OnSave(InterDomainContext context) diff --git a/src/runtime/methodobject.cs b/src/runtime/methodobject.cs index 5fa965f1b..c181c5df1 100644 --- a/src/runtime/methodobject.cs +++ b/src/runtime/methodobject.cs @@ -217,12 +217,11 @@ protected override void Dealloc() base.Dealloc(); } - public static int tp_clear(IntPtr ob) + protected override void Clear() { - var self = (MethodObject)GetManagedObject(ob); - self.ClearMembers(); - ClearObjectDict(ob); - return 0; + this.ClearMembers(); + ClearObjectDict(this.pyHandle); + base.Clear(); } protected override void OnSave(InterDomainContext context) diff --git a/src/runtime/moduleobject.cs b/src/runtime/moduleobject.cs index 8892390c3..7d4992618 100644 --- a/src/runtime/moduleobject.cs +++ b/src/runtime/moduleobject.cs @@ -295,12 +295,6 @@ public static IntPtr tp_repr(IntPtr ob) return Runtime.PyString_FromString($""); } - protected override void Dealloc() - { - tp_clear(this.pyHandle); - base.Dealloc(); - } - public static int tp_traverse(IntPtr ob, IntPtr visit, IntPtr arg) { var self = (ModuleObject)GetManagedObject(ob); @@ -314,17 +308,22 @@ public static int tp_traverse(IntPtr ob, IntPtr visit, IntPtr arg) return 0; } - public static int tp_clear(IntPtr ob) + protected override void Dealloc() { - var self = (ModuleObject)GetManagedObject(ob); - Runtime.Py_CLEAR(ref self.dict); - ClearObjectDict(ob); - foreach (var attr in self.cache.Values) + tp_clear(this.pyHandle); + base.Dealloc(); + } + + protected override void Clear() + { + Runtime.Py_CLEAR(ref this.dict); + ClearObjectDict(this.pyHandle); + foreach (var attr in this.cache.Values) { Runtime.XDecref(attr.pyHandle); } - self.cache.Clear(); - return 0; + this.cache.Clear(); + base.Clear(); } protected override void OnSave(InterDomainContext context) diff --git a/src/runtime/overload.cs b/src/runtime/overload.cs index 48fabca4a..c6c3158fb 100644 --- a/src/runtime/overload.cs +++ b/src/runtime/overload.cs @@ -63,5 +63,11 @@ protected override void Dealloc() Runtime.Py_CLEAR(ref this.target); base.Dealloc(); } + + protected override void Clear() + { + Runtime.Py_CLEAR(ref this.target); + base.Clear(); + } } } From 7271d88fae6d0776a66b92a45c796b518111fab4 Mon Sep 17 00:00:00 2001 From: Victor Nova Date: Sat, 22 May 2021 23:55:52 -0700 Subject: [PATCH 37/39] all Dealloc overrides simply duplicate Clear, so just call both from tp_dealloc and don't override Dealloc --- src/runtime/constructorbinding.cs | 12 ------------ src/runtime/eventbinding.cs | 6 ------ src/runtime/eventobject.cs | 10 ---------- src/runtime/extensiontype.cs | 15 ++++++++++++--- src/runtime/methodbinding.cs | 15 ++------------- src/runtime/methodobject.cs | 21 +++++---------------- src/runtime/moduleobject.cs | 6 ------ src/runtime/overload.cs | 6 ------ 8 files changed, 19 insertions(+), 72 deletions(-) diff --git a/src/runtime/constructorbinding.cs b/src/runtime/constructorbinding.cs index da35aafd1..9ac1adc0f 100644 --- a/src/runtime/constructorbinding.cs +++ b/src/runtime/constructorbinding.cs @@ -149,12 +149,6 @@ public static IntPtr tp_repr(IntPtr ob) return self.repr; } - protected override void Dealloc() - { - Runtime.Py_CLEAR(ref this.repr); - base.Dealloc(); - } - protected override void Clear() { Runtime.Py_CLEAR(ref this.repr); @@ -247,12 +241,6 @@ public static IntPtr tp_repr(IntPtr ob) return self.repr; } - protected override void Dealloc() - { - Runtime.Py_CLEAR(ref this.repr); - base.Dealloc(); - } - protected override void Clear() { Runtime.Py_CLEAR(ref this.repr); diff --git a/src/runtime/eventbinding.cs b/src/runtime/eventbinding.cs index 8fb0ce250..65c8fdccf 100644 --- a/src/runtime/eventbinding.cs +++ b/src/runtime/eventbinding.cs @@ -103,12 +103,6 @@ public static IntPtr tp_repr(IntPtr ob) return Runtime.PyString_FromString(s); } - protected override void Dealloc() - { - Runtime.Py_CLEAR(ref this.target); - base.Dealloc(); - } - protected override void Clear() { Runtime.Py_CLEAR(ref this.target); diff --git a/src/runtime/eventobject.cs b/src/runtime/eventobject.cs index a0f06c375..941bbdf46 100644 --- a/src/runtime/eventobject.cs +++ b/src/runtime/eventobject.cs @@ -198,16 +198,6 @@ public static IntPtr tp_repr(IntPtr ob) } - protected override void Dealloc() - { - if (this.unbound is not null) - { - Runtime.XDecref(this.unbound.pyHandle); - this.unbound = null; - } - base.Dealloc(); - } - protected override void Clear() { if (this.unbound is not null) diff --git a/src/runtime/extensiontype.cs b/src/runtime/extensiontype.cs index 38fe238de..78df805ee 100644 --- a/src/runtime/extensiontype.cs +++ b/src/runtime/extensiontype.cs @@ -56,14 +56,22 @@ void SetupGc () protected virtual void Dealloc() { - ClearObjectDict(this.pyHandle); + var type = Runtime.PyObject_TYPE(this.ObjectReference); Runtime.PyObject_GC_Del(this.pyHandle); - // Not necessary for decref of `tpHandle`. + // Not necessary for decref of `tpHandle` - it is borrowed + this.FreeGCHandle(); + + // we must decref our type: https://docs.python.org/3/c-api/typeobj.html#c.PyTypeObject.tp_dealloc + Runtime.XDecref(type.DangerousGetAddress()); } /// DecRefs and nulls any fields pointing back to Python - protected virtual void Clear() { } + protected virtual void Clear() + { + ClearObjectDict(this.pyHandle); + // Not necessary for decref of `tpHandle` - it is borrowed + } /// /// Type __setattr__ implementation. @@ -96,6 +104,7 @@ public static void tp_dealloc(IntPtr ob) // Clean up a Python instance of this extension type. This // frees the allocated Python object and decrefs the type. var self = (ExtensionType)GetManagedObject(ob); + self?.Clear(); self?.Dealloc(); } diff --git a/src/runtime/methodbinding.cs b/src/runtime/methodbinding.cs index a842dd308..c1e729f9e 100644 --- a/src/runtime/methodbinding.cs +++ b/src/runtime/methodbinding.cs @@ -229,21 +229,10 @@ public static IntPtr tp_repr(IntPtr ob) return Runtime.PyString_FromString($"<{type} method '{name}'>"); } - private void ClearMembers() - { - Runtime.Py_CLEAR(ref target); - Runtime.Py_CLEAR(ref targetType); - } - - protected override void Dealloc() - { - this.ClearMembers(); - base.Dealloc(); - } - protected override void Clear() { - this.ClearMembers(); + Runtime.Py_CLEAR(ref this.target); + Runtime.Py_CLEAR(ref this.targetType); base.Clear(); } diff --git a/src/runtime/methodobject.cs b/src/runtime/methodobject.cs index c181c5df1..2787ec999 100644 --- a/src/runtime/methodobject.cs +++ b/src/runtime/methodobject.cs @@ -200,26 +200,15 @@ public static IntPtr tp_repr(IntPtr ob) return Runtime.PyString_FromString($""); } - private void ClearMembers() + protected override void Clear() { - Runtime.Py_CLEAR(ref doc); - if (unbound != null) + Runtime.Py_CLEAR(ref this.doc); + if (this.unbound != null) { - Runtime.XDecref(unbound.pyHandle); - unbound = null; + Runtime.XDecref(this.unbound.pyHandle); + this.unbound = null; } - } - protected override void Dealloc() - { - this.ClearMembers(); - ClearObjectDict(this.pyHandle); - base.Dealloc(); - } - - protected override void Clear() - { - this.ClearMembers(); ClearObjectDict(this.pyHandle); base.Clear(); } diff --git a/src/runtime/moduleobject.cs b/src/runtime/moduleobject.cs index 7d4992618..dfb6fdf55 100644 --- a/src/runtime/moduleobject.cs +++ b/src/runtime/moduleobject.cs @@ -308,12 +308,6 @@ public static int tp_traverse(IntPtr ob, IntPtr visit, IntPtr arg) return 0; } - protected override void Dealloc() - { - tp_clear(this.pyHandle); - base.Dealloc(); - } - protected override void Clear() { Runtime.Py_CLEAR(ref this.dict); diff --git a/src/runtime/overload.cs b/src/runtime/overload.cs index c6c3158fb..8222dc136 100644 --- a/src/runtime/overload.cs +++ b/src/runtime/overload.cs @@ -58,12 +58,6 @@ public static IntPtr tp_repr(IntPtr op) return doc; } - protected override void Dealloc() - { - Runtime.Py_CLEAR(ref this.target); - base.Dealloc(); - } - protected override void Clear() { Runtime.Py_CLEAR(ref this.target); From 4f3f648d6a57f68ad99236baef26bd4afea56abc Mon Sep 17 00:00:00 2001 From: Victor Milovanov Date: Sat, 29 May 2021 14:49:28 -0700 Subject: [PATCH 38/39] fixup! merge latest master --- src/runtime/Python.Runtime.csproj | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/src/runtime/Python.Runtime.csproj b/src/runtime/Python.Runtime.csproj index c158a19b2..0311dbf9a 100644 --- a/src/runtime/Python.Runtime.csproj +++ b/src/runtime/Python.Runtime.csproj @@ -1,9 +1,9 @@ - + netstandard2.0 AnyCPU 9.0 - Python.Runtime + Python.Runtime Python.Runtime pythonnet @@ -20,30 +20,30 @@ true snupkg - ..\pythonnet.snk + ..\pythonnet.snk true 1591;NU1701 True true - + ..\..\pythonnet\runtime false - + - + - + - + - - clr.py - - + + clr.py + + From c500a3929fb2cedd8c9727cde0e9ce7fdf0a4978 Mon Sep 17 00:00:00 2001 From: Victor Nova Date: Sat, 29 May 2021 14:57:57 -0700 Subject: [PATCH 39/39] fixup! reworked `PythonException`: --- src/runtime/pythonexception.cs | 4 ++-- src/runtime/runtime.cs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/runtime/pythonexception.cs b/src/runtime/pythonexception.cs index 13316aef9..cca7c439f 100644 --- a/src/runtime/pythonexception.cs +++ b/src/runtime/pythonexception.cs @@ -354,8 +354,8 @@ public PythonException Clone() internal bool Is(IntPtr type) { return Runtime.PyErr_GivenExceptionMatches( - (Value ?? Type).Reference, - new BorrowedReference(type)) != 0; + given: (Value ?? Type).Reference, + typeOrTypes: new BorrowedReference(type)) != 0; } private static void CheckRuntimeIsRunning() diff --git a/src/runtime/runtime.cs b/src/runtime/runtime.cs index 5d6a3d192..6086135f5 100644 --- a/src/runtime/runtime.cs +++ b/src/runtime/runtime.cs @@ -2109,7 +2109,7 @@ internal static void PyErr_SetString(IntPtr ob, string message) internal static int PyErr_ExceptionMatches(IntPtr exception) => Delegates.PyErr_ExceptionMatches(exception); - internal static int PyErr_GivenExceptionMatches(BorrowedReference ob, BorrowedReference val) => Delegates.PyErr_GivenExceptionMatches(ob, val); + internal static int PyErr_GivenExceptionMatches(BorrowedReference given, BorrowedReference typeOrTypes) => Delegates.PyErr_GivenExceptionMatches(given, typeOrTypes); internal static void PyErr_NormalizeException(ref NewReference type, ref NewReference val, ref NewReference tb) => Delegates.PyErr_NormalizeException(ref type, ref val, ref tb);