Skip to content

Commit 51c8d90

Browse files
committed
Clear rejected overload probe errors when a method bind succeeds
MethodBinder probes each overload candidate with setError: false, but a probe can still raise: the implicit-conversion catches in Converter.ToManagedValue raise unconditionally so that Invoke can surface the specific cause (e.g. a throwing implicit operator) as the bind-failure reason instead of the generic no-match message. When the raising candidate was probed after the candidate that ultimately matched, nothing cleared the indicator before the method ran and the otherwise successful call failed with "SystemError: ... returned a result with an error set". Errors raised by earlier candidates were incidentally wiped by the per-candidate PyObject_Type/Exceptions.Clear type check, which is why the leak needs this specific overload ordering: an exact-class-match overload (precedence 40) binds first, then a string overload (precedence 50) is probed and raises. Clear any pending probe error in Bind once an overload has matched. The bind-failure path is untouched, so a pending error is still surfaced as the failure reason when nothing matches (ImplicitConversionErrorHandling).
1 parent de1d156 commit 51c8d90

2 files changed

Lines changed: 36 additions & 0 deletions

File tree

src/embed_tests/TestMethodBinder.cs

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,8 @@ def TestG(self):
3535
model.TestList(model.SomeList)
3636
def TestH(self):
3737
return self.OnlyString(TestMethodBinder.ErroredImplicitConversion())
38+
def TestI(self):
39+
return self.StringOrErrored(TestMethodBinder.ErroredImplicitConversion())
3840
def MethodTimeSpanTest(self):
3941
TestMethodBinder.CSharpModel.MethodDateTimeAndTimeSpan(self, timedelta(days = 1), TestMethodBinder.SomeEnu.A, pinocho = 0)
4042
TestMethodBinder.CSharpModel.MethodDateTimeAndTimeSpan(self, date(1, 1, 1), TestMethodBinder.SomeEnu.A, pinocho = 0)
@@ -180,6 +182,22 @@ public void ImplicitConversionErrorHandling()
180182
}
181183
}
182184

185+
// The exact-type overload matches first (class precedence 40), then the
186+
// string overload (precedence 50) is probed and its conversion raises a
187+
// TypeError (throwing implicit operator). The successful bind must not leave
188+
// that probe error pending, or CPython fails the otherwise successful call
189+
// with "SystemError: ... returned a result with an error set".
190+
[Test]
191+
public void RejectedOverloadProbeErrorDoesNotPoisonSuccessfulBind()
192+
{
193+
using (Py.GIL())
194+
{
195+
var data = (string)module.TestI();
196+
Assert.AreEqual("ErroredImplicitConversion overload", data);
197+
Assert.IsFalse(Exceptions.ErrorOccurred());
198+
}
199+
}
200+
183201
[Test]
184202
public void WillAvoidUsingImplicitConversionIfPossible_String()
185203
{
@@ -1533,6 +1551,16 @@ public virtual string OnlyString(string data)
15331551
return "OnlyString impl: " + data;
15341552
}
15351553

1554+
public string StringOrErrored(string data)
1555+
{
1556+
return "string overload";
1557+
}
1558+
1559+
public string StringOrErrored(ErroredImplicitConversion data)
1560+
{
1561+
return "ErroredImplicitConversion overload";
1562+
}
1563+
15361564
public virtual string InvokeModel(string data, double anotherArgument = 0)
15371565
{
15381566
return "string impl: " + data;

src/runtime/MethodBinder.cs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -792,6 +792,14 @@ internal Binding Bind(BorrowedReference inst, BorrowedReference args, BorrowedRe
792792

793793
if (matches.Count > 0 || (matchesUsingImplicitConversion != null && matchesUsingImplicitConversion.Count > 0))
794794
{
795+
// A rejected overload's conversion probe may have raised a Python error
796+
// even though probing converts with setError: false (e.g. a throwing
797+
// implicit operator raises unconditionally so Invoke can surface it as
798+
// the failure reason when nothing matches). Once an overload matched,
799+
// that error must not survive the successful call: CPython would fail
800+
// it with "SystemError: ... returned a result with an error set".
801+
Exceptions.Clear();
802+
795803
// We favor matches that do not use implicit conversion
796804
var matchesTouse = matches.Count > 0 ? matches : matchesUsingImplicitConversion;
797805

0 commit comments

Comments
 (0)