diff --git a/src/embed_tests/TestConverter.cs b/src/embed_tests/TestConverter.cs index 778333366..1355b2247 100644 --- a/src/embed_tests/TestConverter.cs +++ b/src/embed_tests/TestConverter.cs @@ -513,6 +513,39 @@ class TestPythonModel(TestCSharpModel): Assert.AreEqual(shouldConvert, Converter.ToManaged(testPythonModelClass, type, out var result, setError: false)); Assert.IsFalse(Exceptions.ErrorOccurred()); } + + [TestCase(true)] + [TestCase(false)] + public void RaisingIteratorFailsArrayConversion(bool setError) + { + using var _ = Py.GIL(); + + var module = PyModule.FromString("RaisingIteratorModule", @" +def gen(): + yield 1 + raise ValueError('mid-iteration failure') +"); + using var generator = module.GetAttr("gen").Invoke(); + + // the conversion must fail rather than return a silently truncated array, + // and the raised error must only be left pending when setError is true + Assert.IsFalse(Converter.ToManaged(generator, typeof(int[]), out var result, setError)); + Assert.IsNull(result); + Assert.AreEqual(setError, Exceptions.ErrorOccurred()); + Exceptions.Clear(); + } + + [TestCase(true)] + [TestCase(false)] + public void OverflowConversionOnlyLeavesErrorWhenSettingErrors(bool setError) + { + using var _ = Py.GIL(); + + using var pyValue = new PyInt(300); + Assert.IsFalse(Converter.ToManaged(pyValue, typeof(byte), out var _, setError)); + Assert.AreEqual(setError, Exceptions.ErrorOccurred()); + Exceptions.Clear(); + } } public interface IGetList diff --git a/src/embed_tests/TestMethodBinder.cs b/src/embed_tests/TestMethodBinder.cs index 8e41a22de..48a427291 100644 --- a/src/embed_tests/TestMethodBinder.cs +++ b/src/embed_tests/TestMethodBinder.cs @@ -35,6 +35,8 @@ def TestG(self): model.TestList(model.SomeList) def TestH(self): return self.OnlyString(TestMethodBinder.ErroredImplicitConversion()) + def TestI(self): + return self.StringOrErrored(TestMethodBinder.ErroredImplicitConversion()) def MethodTimeSpanTest(self): TestMethodBinder.CSharpModel.MethodDateTimeAndTimeSpan(self, timedelta(days = 1), TestMethodBinder.SomeEnu.A, pinocho = 0) TestMethodBinder.CSharpModel.MethodDateTimeAndTimeSpan(self, date(1, 1, 1), TestMethodBinder.SomeEnu.A, pinocho = 0) @@ -180,6 +182,22 @@ public void ImplicitConversionErrorHandling() } } + // The exact-type overload matches first (class precedence 40), then the + // string overload (precedence 50) is probed and its conversion raises a + // TypeError (throwing implicit operator). The successful bind must not leave + // that probe error pending, or CPython fails the otherwise successful call + // with "SystemError: ... returned a result with an error set". + [Test] + public void RejectedOverloadProbeErrorDoesNotPoisonSuccessfulBind() + { + using (Py.GIL()) + { + var data = (string)module.TestI(); + Assert.AreEqual("ErroredImplicitConversion overload", data); + Assert.IsFalse(Exceptions.ErrorOccurred()); + } + } + [Test] public void WillAvoidUsingImplicitConversionIfPossible_String() { @@ -1533,6 +1551,16 @@ public virtual string OnlyString(string data) return "OnlyString impl: " + data; } + public string StringOrErrored(string data) + { + return "string overload"; + } + + public string StringOrErrored(ErroredImplicitConversion data) + { + return "ErroredImplicitConversion overload"; + } + public virtual string InvokeModel(string data, double anotherArgument = 0) { return "string impl: " + data; diff --git a/src/embed_tests/TestPyObject.cs b/src/embed_tests/TestPyObject.cs index 2a3ebfec4..31e730424 100644 --- a/src/embed_tests/TestPyObject.cs +++ b/src/embed_tests/TestPyObject.cs @@ -102,6 +102,40 @@ public void InheritedMethodsAutoacquireGIL() { PythonEngine.Exec("from System import String\nString.Format('{0},{1}', 1, 2)"); } + + [Test] + public void FailedTryAsDoesNotLeavePythonErrorSet() + { + using var _ = Py.GIL(); + + using var locals = new PyDict(); + PythonEngine.Exec("def a_function(a, b): return a * b", null, locals); + using var pyObject = locals.GetItem("a_function"); + + Assert.IsFalse(pyObject.TryAs(out var _)); + Assert.IsFalse(Exceptions.ErrorOccurred()); + + Assert.IsFalse(pyObject.TryAsManagedObject(typeof(decimal), out var _)); + Assert.IsFalse(Exceptions.ErrorOccurred()); + + // The thread state must be clean for subsequent unrelated Python calls + Assert.DoesNotThrow(() => PyModule.FromString("TryAsLeakCheck", "x = 1").Dispose()); + } + + [Test] + public void FailedAsManagedObjectRaisesWithConversionErrorAsCause() + { + using var _ = Py.GIL(); + + using var locals = new PyDict(); + PythonEngine.Exec("def a_function(a, b): return a * b", null, locals); + using var pyObject = locals.GetItem("a_function"); + + var exception = Assert.Throws(() => pyObject.AsManagedObject(typeof(int))); + Assert.IsNotNull(exception.InnerException); + StringAssert.Contains("int()", exception.InnerException.Message); + Assert.IsFalse(Exceptions.ErrorOccurred()); + } } public class PyObjectTestMethods diff --git a/src/perf_tests/Python.PerformanceTests.csproj b/src/perf_tests/Python.PerformanceTests.csproj index 6d38f74bc..d5f1deda3 100644 --- a/src/perf_tests/Python.PerformanceTests.csproj +++ b/src/perf_tests/Python.PerformanceTests.csproj @@ -13,7 +13,7 @@ runtime; build; native; contentfiles; analyzers; buildtransitive - + compile @@ -25,7 +25,7 @@ - + diff --git a/src/runtime/Converter.cs b/src/runtime/Converter.cs index 3df66c385..5b5416a46 100644 --- a/src/runtime/Converter.cs +++ b/src/runtime/Converter.cs @@ -507,6 +507,8 @@ internal static bool ToManagedValue(BorrowedReference value, Type obType, catch { // Failed to convert using implicit conversion, must catch the error to stop program from exploding on Linux + // Raised even when setError is false: MethodBinder.Invoke surfaces a pending + // error as the binding failure reason instead of its generic no-match message Exceptions.RaiseTypeError($"Failed to implicitly convert {type} to {obType}"); return false; } @@ -711,6 +713,8 @@ internal static bool ToManagedValue(BorrowedReference value, Type obType, catch { // Failed to convert using implicit conversion, must catch the error to stop program from exploding on Linux + // Raised even when setError is false: MethodBinder.Invoke surfaces a pending + // error as the binding failure reason instead of its generic no-match message Exceptions.RaiseTypeError($"Failed to implicitly convert {result.GetType()} to {obType}"); return false; } @@ -1371,6 +1375,11 @@ internal static bool ToPrimitive(BorrowedReference value, Type obType, out objec string tpName = Runtime.PyObject_GetTypeName(value); Exceptions.SetError(Exceptions.TypeError, $"'{tpName}' value cannot be converted to {obType}"); } + else + { + // a probing sub-path may have raised before jumping here + Exceptions.Clear(); + } return false; overflow: @@ -1379,6 +1388,10 @@ internal static bool ToPrimitive(BorrowedReference value, Type obType, out objec { Exceptions.SetError(Exceptions.OverflowError, "value too large to convert"); } + else + { + Exceptions.Clear(); + } return false; } @@ -1456,7 +1469,23 @@ private static bool ToArray(BorrowedReference value, Type obType, out object res private static bool ToList(BorrowedReference value, Type obType, out object result, bool setError) { var elementType = obType.GetGenericArguments()[0]; - var IterObject = Runtime.PyObject_GetIter(value); + result = null; + + using var IterObject = Runtime.PyObject_GetIter(value); + if (IterObject.IsNull()) + { + if (setError) + { + SetConversionError(value, obType); + } + else + { + // PyObject_GetIter will have set an error + Exceptions.Clear(); + } + return false; + } + result = MakeList(value, IterObject, obType, elementType, setError); return result != null; } @@ -1499,6 +1528,11 @@ private static IList MakeList(BorrowedReference value, NewReference IterObject, Exceptions.SetError(e); SetConversionError(value, obType); } + else + { + // PySequence_Size may have raised (e.g. a broken __len__) + Exceptions.Clear(); + } return null; } @@ -1506,10 +1540,29 @@ private static IList MakeList(BorrowedReference value, NewReference IterObject, while (true) { using var item = Runtime.PyIter_Next(IterObject.Borrow()); - if (item.IsNull()) break; + if (item.IsNull()) + { + if (Exceptions.ErrorOccurred()) + { + // the iterator raised mid-iteration: the conversion failed, + // don't return a silently truncated list + if (!setError) + { + Exceptions.Clear(); + } + return null; + } + break; + } if (!Converter.ToManaged(item.Borrow(), elementType, out var obj, setError)) { + // some element conversion paths raise even when setError is false + // (e.g. failed implicit operators, deleted types) + if (!setError) + { + Exceptions.Clear(); + } return null; } diff --git a/src/runtime/MethodBinder.cs b/src/runtime/MethodBinder.cs index cb0a40954..a20624d2b 100644 --- a/src/runtime/MethodBinder.cs +++ b/src/runtime/MethodBinder.cs @@ -792,6 +792,14 @@ internal Binding Bind(BorrowedReference inst, BorrowedReference args, BorrowedRe if (matches.Count > 0 || (matchesUsingImplicitConversion != null && matchesUsingImplicitConversion.Count > 0)) { + // A rejected overload's conversion probe may have raised a Python error + // even though probing converts with setError: false (e.g. a throwing + // implicit operator raises unconditionally so Invoke can surface it as + // the failure reason when nothing matches). Once an overload matched, + // that error must not survive the successful call: CPython would fail + // it with "SystemError: ... returned a result with an error set". + Exceptions.Clear(); + // We favor matches that do not use implicit conversion var matchesTouse = matches.Count > 0 ? matches : matchesUsingImplicitConversion; diff --git a/src/runtime/Properties/AssemblyInfo.cs b/src/runtime/Properties/AssemblyInfo.cs index ab4fddd1d..6cefd7a41 100644 --- a/src/runtime/Properties/AssemblyInfo.cs +++ b/src/runtime/Properties/AssemblyInfo.cs @@ -4,5 +4,5 @@ [assembly: InternalsVisibleTo("Python.EmbeddingTest, PublicKey=00240000048000009400000006020000002400005253413100040000110000005ffd8f49fb44ab0641b3fd8d55e749f716e6dd901032295db641eb98ee46063cbe0d4a1d121ef0bc2af95f8a7438d7a80a3531316e6b75c2dae92fb05a99f03bf7e0c03980e1c3cfb74ba690aca2f3339ef329313bcc5dccced125a4ffdc4531dcef914602cd5878dc5fbb4d4c73ddfbc133f840231343e013762884d6143189")] [assembly: InternalsVisibleTo("Python.Test, PublicKey=00240000048000009400000006020000002400005253413100040000110000005ffd8f49fb44ab0641b3fd8d55e749f716e6dd901032295db641eb98ee46063cbe0d4a1d121ef0bc2af95f8a7438d7a80a3531316e6b75c2dae92fb05a99f03bf7e0c03980e1c3cfb74ba690aca2f3339ef329313bcc5dccced125a4ffdc4531dcef914602cd5878dc5fbb4d4c73ddfbc133f840231343e013762884d6143189")] -[assembly: AssemblyVersion("2.0.62")] -[assembly: AssemblyFileVersion("2.0.62")] +[assembly: AssemblyVersion("2.0.63")] +[assembly: AssemblyFileVersion("2.0.63")] diff --git a/src/runtime/Python.Runtime.csproj b/src/runtime/Python.Runtime.csproj index 359e42944..52dff414f 100644 --- a/src/runtime/Python.Runtime.csproj +++ b/src/runtime/Python.Runtime.csproj @@ -5,7 +5,7 @@ Python.Runtime Python.Runtime QuantConnect.pythonnet - 2.0.62 + 2.0.63 false LICENSE https://github.com/pythonnet/pythonnet diff --git a/src/runtime/PythonTypes/PyObject.cs b/src/runtime/PythonTypes/PyObject.cs index fc3f1001c..0ba3629ba 100644 --- a/src/runtime/PythonTypes/PyObject.cs +++ b/src/runtime/PythonTypes/PyObject.cs @@ -169,7 +169,7 @@ public static PyObject FromManagedObject(object ob) /// public object? AsManagedObject(Type t) { - if (!TryAsManagedObject(t, out var result)) + if (!Converter.ToManaged(obj, t, out var result, setError: true)) { throw new InvalidCastException("cannot convert object to target type", PythonException.FetchCurrentOrNull(out _)); @@ -188,7 +188,15 @@ public static PyObject FromManagedObject(object ob) /// public bool TryAsManagedObject(Type t, out object? result) { - return Converter.ToManaged(obj, t, out result, true); + if (Converter.ToManaged(obj, t, out result, setError: false)) + { + return true; + } + // A failed Try-conversion must not leave a Python error pending on the + // thread: the caller only sees the boolean, so a stale error indicator + // would surface from an unrelated later call on this thread. + Exceptions.Clear(); + return false; } ///