From 83fd8804503e070d5a7985a41cec544178989531 Mon Sep 17 00:00:00 2001 From: Martin Molinero Date: Fri, 16 Oct 2026 12:16:27 -0300 Subject: [PATCH 1/3] Raise TypeError instead of recursing on unsupported operator directions PyType_Ready adds a slot wrapper for the forward and the reflected method of each number slot FixupSlots sets, when the type dict lacks them. A type declaring an operator in one direction only, like x + x, got a __radd__ wrapper that calls the very slot that dispatched to it, so 1.0 + x, 1 + x or sum([x]) recursed until RecursionError instead of raising TypeError. The same happened with [Flags] enums, whose bitwise operators are forward only (1 & flags). The wrappers without a CLR operator behind them are now removed after PyType_Ready, so the slot returns NotImplemented and Python raises its usual TypeError. A Python subclass can still define the missing method. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 1 + src/embed_tests/TestOperator.cs | 100 ++++++++++++++++++++++++++++ src/runtime/TypeManager.cs | 1 + src/runtime/Types/OperatorMethod.cs | 31 +++++++++ 4 files changed, 133 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 83d72a3a0..4f0c51e8c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -92,6 +92,7 @@ Instead, `PyIterable` does that. ### Fixed +- An operation a type supports in one direction only, like `1.0 + x` when `x` only declares `x + x`, raises `TypeError` instead of recursing until `RecursionError` - Fix incorrect dereference of wrapper object in `tp_repr`, which may result in a program crash - Fixed parameterless .NET constructor being silently called when a matching constructor overload is not found ([#238][i238]) - Fix incorrect dereference in params array handling diff --git a/src/embed_tests/TestOperator.cs b/src/embed_tests/TestOperator.cs index 078215077..33b61f677 100644 --- a/src/embed_tests/TestOperator.cs +++ b/src/embed_tests/TestOperator.cs @@ -550,5 +550,105 @@ public void ShiftOperatorOverloads() assert c.Num == a.Num >> b.Num "); } + + public class ForwardOnlyOperable + { + public static ForwardOnlyOperable operator +(ForwardOnlyOperable a, ForwardOnlyOperable b) => a; + } + + public class ReverseOnlyOperable + { + public static ReverseOnlyOperable operator +(double a, ReverseOnlyOperable b) => b; + } + + [Flags] + public enum OperableFlags + { + A = 1, + B = 2 + } + + private const string OperationErrors = @" +def error(operation): + try: + operation() + except Exception as e: + return type(e).__name__ +"; + + [Test] + public void UnsupportedReflectedOperationRaisesTypeError() + { + // the reflected operation has no CLR overload, it used to recurse until RecursionError + using var _ = Py.GIL(); + using var scope = Py.CreateScope(); + scope.Exec(OperationErrors + @" +from Python.EmbeddingTest import TestOperator +x = TestOperator.ForwardOnlyOperable() +supported = error(lambda: x + x), error(lambda: x.__add__(x)) +unsupported = error(lambda: 1.0 + x), error(lambda: 1 + x), error(lambda: sum([x])), error(lambda: x + 1.0) +has_reflected = hasattr(x, '__radd__') +"); + Assert.AreEqual(new string[] { null, null }, scope.Get("supported")); + CollectionAssert.AreEqual(new[] { "TypeError", "TypeError", "TypeError", "TypeError" }, scope.Get("unsupported")); + Assert.IsFalse(scope.Get("has_reflected")); + } + + [Test] + public void UnsupportedForwardOperationRaisesTypeError() + { + // the forward operation has no CLR overload, it used to recurse until RecursionError + using var _ = Py.GIL(); + using var scope = Py.CreateScope(); + scope.Exec(OperationErrors + @" +from Python.EmbeddingTest import TestOperator +y = TestOperator.ReverseOnlyOperable() +supported = error(lambda: 1.0 + y) +unsupported = error(lambda: y + 1.0), error(lambda: y + y) +has_forward = hasattr(y, '__add__') +"); + Assert.IsNull(scope.Get("supported")); + CollectionAssert.AreEqual(new[] { "TypeError", "TypeError" }, scope.Get("unsupported")); + Assert.IsFalse(scope.Get("has_forward")); + } + + [Test] + public void UnsupportedReflectedFlagsOperationRaisesTypeError() + { + // the [Flags] bitwise operators are forward only + using var _ = Py.GIL(); + using var scope = Py.CreateScope(); + scope.Exec(OperationErrors + @" +from Python.EmbeddingTest import TestOperator +flags = TestOperator.OperableFlags.A | TestOperator.OperableFlags.B +supported = error(lambda: flags & TestOperator.OperableFlags.A) +unsupported = error(lambda: 1 & flags), error(lambda: 1 | flags) +"); + Assert.IsNull(scope.Get("supported")); + CollectionAssert.AreEqual(new[] { "TypeError", "TypeError" }, scope.Get("unsupported")); + } + + [Test] + public void PythonSubclassCanStillDefineTheReflectedOperation() + { + using var _ = Py.GIL(); + using var scope = Py.CreateScope(); + scope.Exec(@" +from Python.EmbeddingTest import TestOperator +class Custom(TestOperator.ForwardOnlyOperable): + def __radd__(self, other): + return 'custom' +class Plain(TestOperator.ForwardOnlyOperable): + pass +custom = 1.0 + Custom() +try: + 1.0 + Plain() + plain = None +except TypeError: + plain = 'TypeError' +"); + Assert.AreEqual("custom", scope.Get("custom")); + Assert.AreEqual("TypeError", scope.Get("plain")); + } } } diff --git a/src/runtime/TypeManager.cs b/src/runtime/TypeManager.cs index cc197d32b..f526d87a6 100644 --- a/src/runtime/TypeManager.cs +++ b/src/runtime/TypeManager.cs @@ -295,6 +295,7 @@ internal static void InitializeClass(PyType type, ClassBase impl, Type clrType) { throw PythonException.ThrowLastAsClrException(); } + OperatorMethod.RemoveUnboundSlotWrappers(type, clrType); var dict = Util.ReadRef(type, TypeOffset.tp_dict); string mn = clrType.Namespace ?? ""; diff --git a/src/runtime/Types/OperatorMethod.cs b/src/runtime/Types/OperatorMethod.cs index e905375e0..25b2f2716 100644 --- a/src/runtime/Types/OperatorMethod.cs +++ b/src/runtime/Types/OperatorMethod.cs @@ -127,6 +127,37 @@ public static void FixupSlots(BorrowedReference pyType, Type clrType) } } + /// + /// PyType_Ready adds a slot wrapper for the forward and the reflected method of every number slot + /// set (__add__ and __radd__ for nb_add) the type does not define. With no CLR + /// overload behind them they call the very slot that dispatched to them, so an operation the type does + /// not support, like 1.0 + x when x only declares x + x, recursed until RecursionError. Removing them + /// lets the slot return NotImplemented, so Python raises its usual TypeError. + /// + public static void RemoveUnboundSlotWrappers(BorrowedReference pyType, Type clrType) + { + var dict = Util.ReadRef(pyType, TypeOffset.tp_dict); + var fixedSlots = GetOperatorCandidates(clrType) + .Where(method => OpMethodMap.ContainsKey(method.Name)) + .Select(method => OpMethodMap[method.Name].MethodName) + .Distinct(); + foreach (var pyName in fixedSlots) + { + RemoveSlotWrapper(dict, pyName); + RemoveSlotWrapper(dict, ReversePyMethodName(pyName)); + } + } + + private static void RemoveSlotWrapper(BorrowedReference dict, string name) + { + var member = Runtime.PyDict_GetItemString(dict, name); + // the CLR operators are managed method objects, anything else was added by PyType_Ready + if (!member.IsNull && ManagedType.GetManagedObject(member) == null && Runtime.PyDict_DelItemString(dict, name) != 0) + { + throw PythonException.ThrowLastAsClrException(); + } + } + static IEnumerable GetOperatorCandidates(Type clrType) { const BindingFlags flags = BindingFlags.Public | BindingFlags.Static; From 95f6f97036616e2cebb086af192d23991394a724 Mon Sep 17 00:00:00 2001 From: Martin Molinero Date: Fri, 16 Oct 2026 12:48:15 -0300 Subject: [PATCH 2/3] Only remove CPython's slot wrappers, never failing the type creation The cleanup now deletes a member only when it is a wrapper_descriptor, the object PyType_Ready adds, instead of anything that is not a managed object. It is best effort: a failed delete clears its error and any exception is swallowed, since a wrapper left behind only keeps the previous behavior. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/embed_tests/TestOperator.cs | 20 ++++++++++++++++++ src/runtime/Types/OperatorMethod.cs | 32 +++++++++++++++++++---------- 2 files changed, 41 insertions(+), 11 deletions(-) diff --git a/src/embed_tests/TestOperator.cs b/src/embed_tests/TestOperator.cs index 33b61f677..8254899f0 100644 --- a/src/embed_tests/TestOperator.cs +++ b/src/embed_tests/TestOperator.cs @@ -650,5 +650,25 @@ class Plain(TestOperator.ForwardOnlyOperable): Assert.AreEqual("custom", scope.Get("custom")); Assert.AreEqual("TypeError", scope.Get("plain")); } + + public class ForwardOnlyOperableWithReflectedMethod + { + public static ForwardOnlyOperableWithReflectedMethod operator +(ForwardOnlyOperableWithReflectedMethod a, ForwardOnlyOperableWithReflectedMethod b) => a; + + // a CLR member named like the reflected method, not a slot wrapper + public string __radd__(double other) => "clr"; + } + + [Test] + public void ClrMemberNamedLikeTheReflectedMethodIsKept() + { + using var _ = Py.GIL(); + using var scope = Py.CreateScope(); + scope.Exec(@" +from Python.EmbeddingTest import TestOperator +result = 1.0 + TestOperator.ForwardOnlyOperableWithReflectedMethod() +"); + Assert.AreEqual("clr", scope.Get("result")); + } } } diff --git a/src/runtime/Types/OperatorMethod.cs b/src/runtime/Types/OperatorMethod.cs index 25b2f2716..12f79ab10 100644 --- a/src/runtime/Types/OperatorMethod.cs +++ b/src/runtime/Types/OperatorMethod.cs @@ -136,25 +136,35 @@ public static void FixupSlots(BorrowedReference pyType, Type clrType) /// public static void RemoveUnboundSlotWrappers(BorrowedReference pyType, Type clrType) { - var dict = Util.ReadRef(pyType, TypeOffset.tp_dict); - var fixedSlots = GetOperatorCandidates(clrType) - .Where(method => OpMethodMap.ContainsKey(method.Name)) - .Select(method => OpMethodMap[method.Name].MethodName) - .Distinct(); - foreach (var pyName in fixedSlots) + // best effort: a wrapper left behind only keeps the previous behavior, so nothing here fails the type creation + try { - RemoveSlotWrapper(dict, pyName); - RemoveSlotWrapper(dict, ReversePyMethodName(pyName)); + var dict = Util.ReadRef(pyType, TypeOffset.tp_dict); + var fixedSlots = GetOperatorCandidates(clrType) + .Where(method => OpMethodMap.ContainsKey(method.Name)) + .Select(method => OpMethodMap[method.Name].MethodName) + .Distinct(); + foreach (var pyName in fixedSlots) + { + RemoveSlotWrapper(dict, pyName); + RemoveSlotWrapper(dict, ReversePyMethodName(pyName)); + } + } + catch + { + // pass } } private static void RemoveSlotWrapper(BorrowedReference dict, string name) { var member = Runtime.PyDict_GetItemString(dict, name); - // the CLR operators are managed method objects, anything else was added by PyType_Ready - if (!member.IsNull && ManagedType.GetManagedObject(member) == null && Runtime.PyDict_DelItemString(dict, name) != 0) + // only the slot wrappers PyType_Ready added, the CLR operators are managed method objects + if (!member.IsNull && Runtime.PyObject_TYPE(member) == Runtime.PyWrapperDescriptorType + && Runtime.PyDict_DelItemString(dict, name) != 0) { - throw PythonException.ThrowLastAsClrException(); + // don't let a failed delete leak, the wrapper just stays + Exceptions.Clear(); } } From e5ce9e54fc7df5a823eeabcdab8e4a05883fa6a5 Mon Sep 17 00:00:00 2001 From: Martin Molinero Date: Fri, 16 Oct 2026 18:26:01 -0300 Subject: [PATCH 3/3] Revert the CHANGELOG entry Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 1 - 1 file changed, 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4f0c51e8c..83d72a3a0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -92,7 +92,6 @@ Instead, `PyIterable` does that. ### Fixed -- An operation a type supports in one direction only, like `1.0 + x` when `x` only declares `x + x`, raises `TypeError` instead of recursing until `RecursionError` - Fix incorrect dereference of wrapper object in `tp_repr`, which may result in a program crash - Fixed parameterless .NET constructor being silently called when a matching constructor overload is not found ([#238][i238]) - Fix incorrect dereference in params array handling