Skip to content

Commit de1d156

Browse files
committed
Clear leaked Python errors in converter collection and label paths
Several Converter failure paths could leave a Python error pending even when called with setError: false, relying on the caller to clean up: - ToList did not handle a failed PyObject_GetIter: the raised TypeError survived the call (and the iterator reference leaked). Handle it like ToArray does, disposing the reference and clearing when not setting errors. - MakeList treated a null PyIter_Next result as normal exhaustion, but null also means the iterator raised: the conversion now fails instead of returning a silently truncated list, clearing the error when not setting errors and leaving it pending for the caller when setting errors. The swallowed CLR-exception catch and the element-conversion failure path also clear when not setting errors, since some element conversion paths (failed implicit operators, deleted types) raise regardless of setError to enrich MethodBinder's no-match message. - The type_error and overflow labels now clear pending errors when not setting errors, mirroring what convert_error already did, so a probing sub-path that raised before jumping cannot leak. The unconditional raises in the implicit-conversion catches are kept: MethodBinder.Invoke deliberately surfaces a pending error as the binding failure reason instead of its generic no-match message, and Converter.TryAsManagedObject clears at the API boundary.
1 parent 6bfc85c commit de1d156

2 files changed

Lines changed: 88 additions & 2 deletions

File tree

‎src/embed_tests/TestConverter.cs‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -513,6 +513,39 @@ class TestPythonModel(TestCSharpModel):
513513
Assert.AreEqual(shouldConvert, Converter.ToManaged(testPythonModelClass, type, out var result, setError: false));
514514
Assert.IsFalse(Exceptions.ErrorOccurred());
515515
}
516+
517+
[TestCase(true)]
518+
[TestCase(false)]
519+
public void RaisingIteratorFailsArrayConversion(bool setError)
520+
{
521+
using var _ = Py.GIL();
522+
523+
var module = PyModule.FromString("RaisingIteratorModule", @"
524+
def gen():
525+
yield 1
526+
raise ValueError('mid-iteration failure')
527+
");
528+
using var generator = module.GetAttr("gen").Invoke();
529+
530+
// the conversion must fail rather than return a silently truncated array,
531+
// and the raised error must only be left pending when setError is true
532+
Assert.IsFalse(Converter.ToManaged(generator, typeof(int[]), out var result, setError));
533+
Assert.IsNull(result);
534+
Assert.AreEqual(setError, Exceptions.ErrorOccurred());
535+
Exceptions.Clear();
536+
}
537+
538+
[TestCase(true)]
539+
[TestCase(false)]
540+
public void OverflowConversionOnlyLeavesErrorWhenSettingErrors(bool setError)
541+
{
542+
using var _ = Py.GIL();
543+
544+
using var pyValue = new PyInt(300);
545+
Assert.IsFalse(Converter.ToManaged(pyValue, typeof(byte), out var _, setError));
546+
Assert.AreEqual(setError, Exceptions.ErrorOccurred());
547+
Exceptions.Clear();
548+
}
516549
}
517550

518551
public interface IGetList

‎src/runtime/Converter.cs‎

Lines changed: 55 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -507,6 +507,8 @@ internal static bool ToManagedValue(BorrowedReference value, Type obType,
507507
catch
508508
{
509509
// Failed to convert using implicit conversion, must catch the error to stop program from exploding on Linux
510+
// Raised even when setError is false: MethodBinder.Invoke surfaces a pending
511+
// error as the binding failure reason instead of its generic no-match message
510512
Exceptions.RaiseTypeError($"Failed to implicitly convert {type} to {obType}");
511513
return false;
512514
}
@@ -711,6 +713,8 @@ internal static bool ToManagedValue(BorrowedReference value, Type obType,
711713
catch
712714
{
713715
// Failed to convert using implicit conversion, must catch the error to stop program from exploding on Linux
716+
// Raised even when setError is false: MethodBinder.Invoke surfaces a pending
717+
// error as the binding failure reason instead of its generic no-match message
714718
Exceptions.RaiseTypeError($"Failed to implicitly convert {result.GetType()} to {obType}");
715719
return false;
716720
}
@@ -1371,6 +1375,11 @@ internal static bool ToPrimitive(BorrowedReference value, Type obType, out objec
13711375
string tpName = Runtime.PyObject_GetTypeName(value);
13721376
Exceptions.SetError(Exceptions.TypeError, $"'{tpName}' value cannot be converted to {obType}");
13731377
}
1378+
else
1379+
{
1380+
// a probing sub-path may have raised before jumping here
1381+
Exceptions.Clear();
1382+
}
13741383
return false;
13751384

13761385
overflow:
@@ -1379,6 +1388,10 @@ internal static bool ToPrimitive(BorrowedReference value, Type obType, out objec
13791388
{
13801389
Exceptions.SetError(Exceptions.OverflowError, "value too large to convert");
13811390
}
1391+
else
1392+
{
1393+
Exceptions.Clear();
1394+
}
13821395
return false;
13831396
}
13841397

@@ -1456,7 +1469,23 @@ private static bool ToArray(BorrowedReference value, Type obType, out object res
14561469
private static bool ToList(BorrowedReference value, Type obType, out object result, bool setError)
14571470
{
14581471
var elementType = obType.GetGenericArguments()[0];
1459-
var IterObject = Runtime.PyObject_GetIter(value);
1472+
result = null;
1473+
1474+
using var IterObject = Runtime.PyObject_GetIter(value);
1475+
if (IterObject.IsNull())
1476+
{
1477+
if (setError)
1478+
{
1479+
SetConversionError(value, obType);
1480+
}
1481+
else
1482+
{
1483+
// PyObject_GetIter will have set an error
1484+
Exceptions.Clear();
1485+
}
1486+
return false;
1487+
}
1488+
14601489
result = MakeList(value, IterObject, obType, elementType, setError);
14611490
return result != null;
14621491
}
@@ -1499,17 +1528,41 @@ private static IList MakeList(BorrowedReference value, NewReference IterObject,
14991528
Exceptions.SetError(e);
15001529
SetConversionError(value, obType);
15011530
}
1531+
else
1532+
{
1533+
// PySequence_Size may have raised (e.g. a broken __len__)
1534+
Exceptions.Clear();
1535+
}
15021536

15031537
return null;
15041538
}
15051539

15061540
while (true)
15071541
{
15081542
using var item = Runtime.PyIter_Next(IterObject.Borrow());
1509-
if (item.IsNull()) break;
1543+
if (item.IsNull())
1544+
{
1545+
if (Exceptions.ErrorOccurred())
1546+
{
1547+
// the iterator raised mid-iteration: the conversion failed,
1548+
// don't return a silently truncated list
1549+
if (!setError)
1550+
{
1551+
Exceptions.Clear();
1552+
}
1553+
return null;
1554+
}
1555+
break;
1556+
}
15101557

15111558
if (!Converter.ToManaged(item.Borrow(), elementType, out var obj, setError))
15121559
{
1560+
// some element conversion paths raise even when setError is false
1561+
// (e.g. failed implicit operators, deleted types)
1562+
if (!setError)
1563+
{
1564+
Exceptions.Clear();
1565+
}
15131566
return null;
15141567
}
15151568

0 commit comments

Comments
 (0)