Clear pending Python error when a Try-style conversion fails#138
Merged
jhonabreul merged 4 commits intoJul 14, 2026
Merged
Conversation
PyObject.TryAsManagedObject converted with setError: true and returned false on failure, leaving the Python error indicator set on the thread with no one left to surface it. Callers of TryAs/TryAsManagedObject only observe the boolean, so the stale error survived on the thread state and was thrown by the next unrelated Python call that checks the error indicator. This was previously masked because the outermost Py.GIL scope deleted the thread state (and its pending error) on dispose; since thread states are pinned for the lifetime of the run (QuantConnect#137), the leaked error persists and poisons subsequent calls on the same thread - e.g. Lean's PythonUtilTests failing with "int() argument must be a string, a bytes-like object or a real number, not 'function'" leaked by an earlier test that exercises a failed TryAs<int> on a function. Convert with setError: false in TryAsManagedObject and clear any error a conversion sub-path may have left pending before returning false. AsManagedObject keeps converting with setError: true so the fetched error still becomes the InvalidCastException cause, which also consumes the indicator.
Bump package <Version>, AssemblyVersion/AssemblyFileVersion and the perf-test baseline reference to 2.0.63.
11 tasks
Martin-Molinero
approved these changes
Jul 13, 2026
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.
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).
c1a0231 to
51c8d90
Compare
Martin-Molinero
approved these changes
Jul 14, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this implement/fix?
PyObject.TryAsManagedObject(and thereforeTryAs<T>/TrySafeAs<T>) converted withsetError: trueand returnedfalseon failure, leaving the Python error indicator set with no one left to surface it. The stale error was then thrown by the next unrelated Python call on that thread. This was masked while the outermostPy.GILscope deleted the thread state on dispose; since #137 pins thread states, the leaked error persists. It is the root cause of thePythonUtilTests.BothInheritedAndNonInheritedClassesWork(True)CI failure in QuantConnect/Lean#9614: an earlier test's failedTryAs<int>on a Python function left aTypeErrorpending that the next Python-touching test then threw.The fix:
TryAsManagedObjectconverts withsetError: falseand clears anything a conversion sub-path may have left pending before returningfalse.AsManagedObjectkeepssetError: true— fetching the error for theInvalidCastExceptioncause consumes the indicator. Also bumps the version to 2.0.63.Two follow-up commits close the same class of leak elsewhere:
ToListhandles a failedPyObject_GetIterlikeToArraydoes (also fixing a leaked iterator reference);MakeListfails the conversion when the iterator raises mid-iteration instead of returning a silently truncated list, and clears swallowed errors when not setting errors; thetype_error/overflowlabels clear pending errors when not setting errors, mirroringconvert_error.setError: falsebut can still raise (see below). When the raising candidate was probed after the candidate that matched, the pending error survived into the successful call, which then failed withSystemError: ... returned a result with an error set.Bindnow clears any pending probe error once an overload has matched; the no-match path is untouched.Why some conversion paths deliberately raise even with
setError: false, and why they stay that wayThe two implicit-conversion catch blocks in
Converter.ToManagedValuecallExceptions.RaiseTypeError("Failed to implicitly convert {source} to {target}")unconditionally, ignoringsetError. This is intentional and this PR keeps it:MethodBinderprobes every overload withsetError: false. When no overload matches,MethodBinder.Invokechecks the error indicator and, if a probe left an error, surfaces that instead of the generic "No method matches given arguments" message. A throwing implicit operator is the canonical case: without the unconditional raise, the actionable cause ("Failed to implicitly convert X to Y") would be silently discarded during probing and the user would only see the generic no-match error. This contract is covered byImplicitConversionErrorHandling.setErrorwould therefore degrade diagnostics: probing always runs withsetError: false, so the specific cause could never reach the user.The consequence is that "converted with
setError: false" does not guarantee "no error pending", so each API boundary that swallows a failed conversion is responsible for clearing:TryAsManagedObjectclears before returningfalse,MakeListclears its element-conversion failures, andMethodBinder.Bindnow clears once an overload has matched (keeping the error only on the no-match path, where it is the diagnostic).Does this close any currently open issues?
Fixes the
PythonUtilTests.BothInheritedAndNonInheritedClassesWorkCI failure in QuantConnect/Lean#9614.Any other comments?
Regression tests added:
FailedTryAsDoesNotLeavePythonErrorSet— a failedTryAs<int>leaves no pending error; subsequentPyModule.FromStringcalls work.FailedAsManagedObjectRaisesWithConversionErrorAsCause—AsManagedObjectstill raisesInvalidCastExceptionwith the Python error as its cause.RaisingIteratorFailsArrayConversion/OverflowConversionOnlyLeavesErrorWhenSettingErrors— these conversions fail leaving an error pending only whensetErroris true.RejectedOverloadProbeErrorDoesNotPoisonSuccessfulBind— a rejected overload's raising probe (throwing implicit operator) does not poison a successful bind of a sibling overload.Verified end-to-end against Lean: 2.0.62 reproduces the exact CI failure when running
ExtensionsTests+PythonUtilTestson a single NUnit worker; with this build all 352 tests pass. Full embedding test suite: 965 passed, 8 skipped. A full Lean test-suite sweep with a per-test PyErr-leak detector found theTryAspath to be the only leak source across 36,316 tests.Checklist
Check all those that are applicable and complete.
AUTHORSCHANGELOG