From b988292482a124c38800b7c8f60932589a076163 Mon Sep 17 00:00:00 2001 From: Victor Nova Date: Thu, 3 Mar 2022 14:18:17 -0800 Subject: [PATCH] make methods of PyObject inherited from its base C# classes GIL-safe fixes https://github.com/pythonnet/pythonnet/issues/1642 --- CHANGELOG.md | 2 ++ src/embed_tests/Codecs.cs | 2 +- src/embed_tests/TestPyObject.cs | 7 +++++++ src/runtime/InternString.cs | 2 +- src/runtime/Py.cs | 10 +--------- src/runtime/PythonTypes/PyObject.cs | 21 ++++++++++++++++----- 6 files changed, 28 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 382f9ab57..20b303dc6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,7 @@ details about the cause of the failure - floating point values passed from Python are no longer silently truncated when .NET expects an integer [#1342][i1342] - More specific error messages for method argument mismatch +- members of `PyObject` inherited from `System.Object and `DynamicObject` now autoacquire GIL - BREAKING: when inheriting from .NET types in Python if you override `__init__` you must explicitly call base constructor using `super().__init__(.....)`. Not doing so will lead to undefined behavior. @@ -69,6 +70,7 @@ One must now either use enum members (e.g. `MyEnum.Option`), or use enum constru - BREAKING: Names of .NET types (e.g. `str(__class__)`) changed to better support generic types - BREAKING: overload resolution will no longer prefer basic types. Instead, first matching overload will be chosen. +- BREAKING: acquiring GIL using `Py.GIL` no longer forces `PythonEngine` to initialize - BREAKING: `Exec` and `Eval` from `PythonEngine` no longer accept raw pointers. - BREAKING: .NET collections and arrays are no longer automatically converted to Python collections. Instead, they implement standard Python diff --git a/src/embed_tests/Codecs.cs b/src/embed_tests/Codecs.cs index a87b287bc..c9e83f03a 100644 --- a/src/embed_tests/Codecs.cs +++ b/src/embed_tests/Codecs.cs @@ -473,7 +473,7 @@ public DecoderReturningPredefinedValue(PyObject objectType, TTarget decodeResult } public bool CanDecode(PyType objectType, Type targetType) - => objectType.Handle == TheOnlySupportedSourceType.Handle + => PythonReferenceComparer.Instance.Equals(objectType, TheOnlySupportedSourceType) && targetType == typeof(TTarget); public bool TryDecode(PyObject pyObj, out T value) { diff --git a/src/embed_tests/TestPyObject.cs b/src/embed_tests/TestPyObject.cs index fa5fa38c7..2f27eba1b 100644 --- a/src/embed_tests/TestPyObject.cs +++ b/src/embed_tests/TestPyObject.cs @@ -94,6 +94,13 @@ public void GetAttrDefault_IgnoresAttributeErrorOnly() ); Assert.AreEqual(Exceptions.TypeError, typeErrResult.Type); } + + // regression test from https://github.com/pythonnet/pythonnet/issues/1642 + [Test] + public void InheritedMethodsAutoacquireGIL() + { + PythonEngine.Exec("from System import String\nString.Format('{0},{1}', 1, 2)"); + } } public class PyObjectTestMethods diff --git a/src/runtime/InternString.cs b/src/runtime/InternString.cs index a479f3732..0780a0bb8 100644 --- a/src/runtime/InternString.cs +++ b/src/runtime/InternString.cs @@ -39,7 +39,7 @@ public static void Initialize() { NewReference pyStr = Runtime.PyUnicode_InternFromString(name); var op = new PyString(pyStr.StealOrThrow()); - Debug.Assert(name == op.ToString()); + Debug.Assert(name == op.As()); SetIntern(name, op); var field = type.GetField("f" + name, PyIdentifierFieldFlags)!; field.SetValue(null, op.rawPtr); diff --git a/src/runtime/Py.cs b/src/runtime/Py.cs index 7a2369413..4f3fbf6d4 100644 --- a/src/runtime/Py.cs +++ b/src/runtime/Py.cs @@ -10,15 +10,7 @@ namespace Python.Runtime; public static class Py { - public static GILState GIL() - { - if (!PythonEngine.IsInitialized) - { - PythonEngine.Initialize(); - } - - return PythonEngine.DebugGIL ? new DebugGILState() : new GILState(); - } + public static GILState GIL() => PythonEngine.DebugGIL ? new DebugGILState() : new GILState(); public static PyModule CreateScope() => new(); public static PyModule CreateScope(string name) diff --git a/src/runtime/PythonTypes/PyObject.cs b/src/runtime/PythonTypes/PyObject.cs index e26831490..e0a17bed5 100644 --- a/src/runtime/PythonTypes/PyObject.cs +++ b/src/runtime/PythonTypes/PyObject.cs @@ -1056,6 +1056,7 @@ public PyList Dir() /// public override string? ToString() { + using var _ = Py.GIL(); using var strval = Runtime.PyObject_Str(obj); return Runtime.GetManagedString(strval.BorrowOrThrow()); } @@ -1072,7 +1073,11 @@ public PyList Dir() /// Return true if this object is equal to the given object. This /// method is based on Python equality semantics. /// - public override bool Equals(object o) => Equals(o as PyObject); + public override bool Equals(object o) + { + using var _ = Py.GIL(); + return Equals(o as PyObject); + } public virtual bool Equals(PyObject? other) { @@ -1101,6 +1106,7 @@ public virtual bool Equals(PyObject? other) /// public override int GetHashCode() { + using var _ = Py.GIL(); nint pyHash = Runtime.PyObject_Hash(obj); if (pyHash == -1 && Exceptions.ErrorOccurred()) { @@ -1135,12 +1141,14 @@ public long Refcount public override bool TryGetMember(GetMemberBinder binder, out object? result) { + using var _ = Py.GIL(); result = CheckNone(this.GetAttr(binder.Name)); return true; } public override bool TrySetMember(SetMemberBinder binder, object? value) { + using var _ = Py.GIL(); using var newVal = Converter.ToPythonDetectType(value); int r = Runtime.PyObject_SetAttrString(obj, binder.Name, newVal.Borrow()); if (r < 0) @@ -1234,6 +1242,7 @@ private static NewReference GetPythonObject(object? target) public override bool TryInvokeMember(InvokeMemberBinder binder, object?[] args, out object? result) { + using var _ = Py.GIL(); if (this.HasAttr(binder.Name) && this.GetAttr(binder.Name).IsCallable()) { PyTuple? pyargs = null; @@ -1258,6 +1267,7 @@ public override bool TryInvokeMember(InvokeMemberBinder binder, object?[] args, public override bool TryInvoke(InvokeBinder binder, object?[] args, out object? result) { + using var _ = Py.GIL(); if (this.IsCallable()) { PyTuple? pyargs = null; @@ -1282,6 +1292,7 @@ public override bool TryInvoke(InvokeBinder binder, object?[] args, out object? public override bool TryConvert(ConvertBinder binder, out object? result) { + using var _ = Py.GIL(); // always try implicit conversion first if (Converter.ToManaged(this.obj, binder.Type, out result, false)) { @@ -1307,6 +1318,7 @@ public override bool TryConvert(ConvertBinder binder, out object? result) public override bool TryBinaryOperation(BinaryOperationBinder binder, object arg, out object? result) { + using var _ = Py.GIL(); NewReference res; if (!(arg is PyObject)) { @@ -1419,6 +1431,7 @@ public override bool TryBinaryOperation(BinaryOperationBinder binder, object arg public override bool TryUnaryOperation(UnaryOperationBinder binder, out object? result) { + using var _ = Py.GIL(); int r; NewReference res; switch (binder.Operation) @@ -1463,10 +1476,8 @@ public override bool TryUnaryOperation(UnaryOperationBinder binder, out object? /// A sequence that contains dynamic member names. public override IEnumerable GetDynamicMemberNames() { - foreach (PyObject pyObj in Dir()) - { - yield return pyObj.ToString()!; - } + using var _ = Py.GIL(); + return Dir().Select(pyObj => pyObj.ToString()!).ToArray(); } void ISerializable.GetObjectData(SerializationInfo info, StreamingContext context)