make methods of PyObject inherited from its base C# classes GIL-safe

lostmsu · lostmsu · commit eec30c062f93 · 2022-03-03T21:58:12.000-08:00
fixes #1642
diff --git 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
@@ -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<T>(PyObject pyObj, out T value)
         {
diff --git 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
@@ -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<string>());
                 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
@@ -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
@@ -1056,6 +1056,7 @@ public PyList Dir()
         /// </remarks>
         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.
         /// </remarks>
-        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)
         /// </remarks>
         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?
         /// <returns>A sequence that contains dynamic member names.</returns>
         public override IEnumerable<string> 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)

Original file line number	Diff line number	Diff line change
`@@ -473,7 +473,7 @@ public DecoderReturningPredefinedValue(PyObject objectType, TTarget decodeResult`
`473`	`473`	`}`
`474`	`474`
`475`	`475`	`public bool CanDecode(PyType objectType, Type targetType)`
`476`		`- => objectType.Handle == TheOnlySupportedSourceType.Handle`
	`476`	`+ => PythonReferenceComparer.Instance.Equals(objectType, TheOnlySupportedSourceType)`
`477`	`477`	`&& targetType == typeof(TTarget);`
`478`	`478`	`public bool TryDecode<T>(PyObject pyObj, out T value)`
`479`	`479`	`{`
Original file line number	Diff line number	Diff line change
`@@ -94,6 +94,13 @@ public void GetAttrDefault_IgnoresAttributeErrorOnly()`
`94`	`94`	`);`
`95`	`95`	`Assert.AreEqual(Exceptions.TypeError, typeErrResult.Type);`
`96`	`96`	`}`
	`97`	`+`
	`98`	`+ // regression test from https://github.com/pythonnet/pythonnet/issues/1642`
	`99`	`+ [Test]`
	`100`	`+ public void InheritedMethodsAutoacquireGIL()`
	`101`	`+ {`
	`102`	`+ PythonEngine.Exec("from System import String\nString.Format('{0},{1}', 1, 2)");`
	`103`	`+ }`
`97`	`104`	`}`
`98`	`105`
`99`	`106`	`public class PyObjectTestMethods`
Original file line number	Diff line number	Diff line change
`@@ -39,7 +39,7 @@ public static void Initialize()`
`39`	`39`	`{`
`40`	`40`	`NewReference pyStr = Runtime.PyUnicode_InternFromString(name);`
`41`	`41`	`var op = new PyString(pyStr.StealOrThrow());`
`42`		`- Debug.Assert(name == op.ToString());`
	`42`	`+ Debug.Assert(name == op.As<string>());`
`43`	`43`	`SetIntern(name, op);`
`44`	`44`	`var field = type.GetField("f" + name, PyIdentifierFieldFlags)!;`
`45`	`45`	`field.SetValue(null, op.rawPtr);`
Original file line number	Diff line number	Diff line change
`@@ -1056,6 +1056,7 @@ public PyList Dir()`
`1056`	`1056`	`/// </remarks>`
`1057`	`1057`	`public override string? ToString()`
`1058`	`1058`	`{`
	`1059`	`+ using var _ = Py.GIL();`
`1059`	`1060`	`using var strval = Runtime.PyObject_Str(obj);`
`1060`	`1061`	`return Runtime.GetManagedString(strval.BorrowOrThrow());`
`1061`	`1062`	`}`
`@@ -1072,7 +1073,11 @@ public PyList Dir()`
`1072`	`1073`	`/// Return true if this object is equal to the given object. This`
`1073`	`1074`	`/// method is based on Python equality semantics.`
`1074`	`1075`	`/// </remarks>`
`1075`		`- public override bool Equals(object o) => Equals(o as PyObject);`
	`1076`	`+ public override bool Equals(object o)`
	`1077`	`+ {`
	`1078`	`+ using var _ = Py.GIL();`
	`1079`	`+ return Equals(o as PyObject);`
	`1080`	`+ }`
`1076`	`1081`
`1077`	`1082`	`public virtual bool Equals(PyObject? other)`
`1078`	`1083`	`{`
`@@ -1101,6 +1106,7 @@ public virtual bool Equals(PyObject? other)`
`1101`	`1106`	`/// </remarks>`
`1102`	`1107`	`public override int GetHashCode()`
`1103`	`1108`	`{`
	`1109`	`+ using var _ = Py.GIL();`
`1104`	`1110`	`nint pyHash = Runtime.PyObject_Hash(obj);`
`1105`	`1111`	`if (pyHash == -1 && Exceptions.ErrorOccurred())`
`1106`	`1112`	`{`
`@@ -1135,12 +1141,14 @@ public long Refcount`
`1135`	`1141`
`1136`	`1142`	`public override bool TryGetMember(GetMemberBinder binder, out object? result)`
`1137`	`1143`	`{`
	`1144`	`+ using var _ = Py.GIL();`
`1138`	`1145`	`result = CheckNone(this.GetAttr(binder.Name));`
`1139`	`1146`	`return true;`
`1140`	`1147`	`}`
`1141`	`1148`
`1142`	`1149`	`public override bool TrySetMember(SetMemberBinder binder, object? value)`
`1143`	`1150`	`{`
	`1151`	`+ using var _ = Py.GIL();`
`1144`	`1152`	`using var newVal = Converter.ToPythonDetectType(value);`
`1145`	`1153`	`int r = Runtime.PyObject_SetAttrString(obj, binder.Name, newVal.Borrow());`
`1146`	`1154`	`if (r < 0)`
`@@ -1234,6 +1242,7 @@ private static NewReference GetPythonObject(object? target)`
`1234`	`1242`
`1235`	`1243`	`public override bool TryInvokeMember(InvokeMemberBinder binder, object?[] args, out object? result)`
`1236`	`1244`	`{`
	`1245`	`+ using var _ = Py.GIL();`
`1237`	`1246`	`if (this.HasAttr(binder.Name) && this.GetAttr(binder.Name).IsCallable())`
`1238`	`1247`	`{`
`1239`	`1248`	`PyTuple? pyargs = null;`
`@@ -1258,6 +1267,7 @@ public override bool TryInvokeMember(InvokeMemberBinder binder, object?[] args,`
`1258`	`1267`
`1259`	`1268`	`public override bool TryInvoke(InvokeBinder binder, object?[] args, out object? result)`
`1260`	`1269`	`{`
	`1270`	`+ using var _ = Py.GIL();`
`1261`	`1271`	`if (this.IsCallable())`
`1262`	`1272`	`{`
`1263`	`1273`	`PyTuple? pyargs = null;`
`@@ -1282,6 +1292,7 @@ public override bool TryInvoke(InvokeBinder binder, object?[] args, out object?`
`1282`	`1292`
`1283`	`1293`	`public override bool TryConvert(ConvertBinder binder, out object? result)`
`1284`	`1294`	`{`
	`1295`	`+ using var _ = Py.GIL();`
`1285`	`1296`	`// always try implicit conversion first`
`1286`	`1297`	`if (Converter.ToManaged(this.obj, binder.Type, out result, false))`
`1287`	`1298`	`{`
`@@ -1307,6 +1318,7 @@ public override bool TryConvert(ConvertBinder binder, out object? result)`
`1307`	`1318`
`1308`	`1319`	`public override bool TryBinaryOperation(BinaryOperationBinder binder, object arg, out object? result)`
`1309`	`1320`	`{`
	`1321`	`+ using var _ = Py.GIL();`
`1310`	`1322`	`NewReference res;`
`1311`	`1323`	`if (!(arg is PyObject))`
`1312`	`1324`	`{`
`@@ -1419,6 +1431,7 @@ public override bool TryBinaryOperation(BinaryOperationBinder binder, object arg`
`1419`	`1431`
`1420`	`1432`	`public override bool TryUnaryOperation(UnaryOperationBinder binder, out object? result)`
`1421`	`1433`	`{`
	`1434`	`+ using var _ = Py.GIL();`
`1422`	`1435`	`int r;`
`1423`	`1436`	`NewReference res;`
`1424`	`1437`	`switch (binder.Operation)`
`@@ -1463,10 +1476,8 @@ public override bool TryUnaryOperation(UnaryOperationBinder binder, out object?`
`1463`	`1476`	`/// <returns>A sequence that contains dynamic member names.</returns>`
`1464`	`1477`	`public override IEnumerable<string> GetDynamicMemberNames()`
`1465`	`1478`	`{`
`1466`		`- foreach (PyObject pyObj in Dir())`
`1467`		`- {`
`1468`		`- yield return pyObj.ToString()!;`
`1469`		`- }`
	`1479`	`+ using var _ = Py.GIL();`
	`1480`	`+ return Dir().Select(pyObj => pyObj.ToString()!).ToArray();`
`1470`	`1481`	`}`
`1471`	`1482`
`1472`	`1483`	`void ISerializable.GetObjectData(SerializationInfo info, StreamingContext context)`