Repository navigation
Fix deadlock when threads first resolve lazy Python members concurrently - #154
Conversation
PyDecimalType and the converter's datetime/tzinfo constructors were Lazy<T> in ExecutionAndPublication mode. Their factories run Python code with the GIL held, and that code can hand the GIL to another thread. A thread that then reached the same Lazy blocked on its lock while holding the GIL, so the factory could never finish: a deadlock. PublicationOnly lets that thread run the factory itself; the first published value wins. Closes #153 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
There was a problem hiding this comment.
Approving. This fixes the root cause of #153: the factory no longer holds a lock while it runs Python code that can hand off the GIL. It's the smallest fix that does that, three one-line changes.
What I checked
- The mechanism. With
ExecutionAndPublication, the second thread waits on theLazy's monitor while it still holds the GIL. WithPublicationOnlyit runs the factory itself instead. For_pydecimal, it then waits on Python's per-module import lock, which releases the GIL. The finder'sfind_specruns inside that lock, so the test exercises exactly this path. Every caller still gets the single published value. - Running a factory twice is safe.
SetPyMemberTypeOfcalls_pyRefs.Addwith the GIL held and nothing in between can release it, so the two adds can't race. The losingPyTypeis still disposed on shutdown with the other refs.- For the converter lazies, a second
PyModule.FromString("clr_datetime"/"custom_tzinfo", …)reuses the same entry insys.modulesand only rebinds equivalent classes.__reduce_ex__pickles to the basedatetime, so nothing depends on which module object wins.
- Exceptions are no longer cached.
PublicationOnlydoesn't cache a factory exception, so a failed first resolution is retried instead of failing every later call. That's no worse than today. - The test fails without the fix. I ran
TestLazyPyMemberslocally (Python 3.11, .NET 10). It passes with the PR. With onlyRuntime.csreverted to master, it hung until my timeout killed it. Five more runs of it together withTestConverterall passed (58/58 each time), so it isn't flaky. - CI is green: the build and tests on Python 3.8 to 3.14, plus the Lean Python regression and unit test jobs.
- Evidence and scope. The description reports a full local suite run and a Lean backtest end to end with eight concurrent first callers. The scope matches the issue.
Not blocking, follow-up only: GetModuleLazy (Runtime.cs:375) builds inspect, clrInterop and hexCallable with isThreadSafe: false, and their factories also import Python modules with the GIL held. With LazyThreadSafetyMode.None, a second thread that reaches the value while the factory is still running gets InvalidOperationException, and the Lazy caches that exception. I haven't reproduced this. If it's reachable concurrently, it belongs in its own issue and PR, not this one.
Other repositories: Lean only picks this up after a QuantConnect.pythonnet release and the matching package bump in Lean (currently 2.0.67).
Risk: I set risk:low. The change is three lines and easy to revert. A test reproduces the deadlock and fails without the fix, CI is green including the Lean jobs, and the description reports an end-to-end run. Behavior without contention is unchanged.
Closes #153
What changed and why
Runtime.PyDecimalTypeand the converter'sdatetime/tzinfoconstructors wereLazy<T>with the defaultExecutionAndPublicationmode. Their factories run Python code (importing_pydecimal, executing small modules) while the calling thread holds the GIL, and that code can hand the GIL to another thread. If that thread then reaches the sameLazy(e.g.Converter.GetTypeByAliasduring overload resolution), it blocks on theLazy's lock while holding the GIL, and the first thread can never get the GIL back to finish. The two threads deadlock.The three lazies now use
LazyThreadSafetyMode.PublicationOnly, so no lock is held while the factory runs. A second thread runs the factory itself (for the import, Python's own per-module import lock makes it wait with the GIL released), and every caller gets the first published value. The factories already run under the GIL, so running one twice is safe:_pydecimalcomes fromsys.modules, so theDecimaltype is the same object, and a duplicatedatetime/GMTclass is simply discarded. Behavior when there's no contention is unchanged.Tests
TestLazyPyMembers.ConcurrentFirstResolutionOfDecimalTypeDoesNotDeadlock. Asys.meta_pathfinder holds the first thread's_pydecimalimport open with the GIL released, so the second thread is guaranteed to reachPyDecimalType(through an overloaded .NET call) while the factory is still running. It passes with the change. Without the change (onlyRuntime.csreverted), the test hangs.Converter's static constructor, so the suite can't run them uncreated. They got the same one-line change for the same reason.dotnet test src/embed_tests/ --filter "FullyQualifiedName!~SetPythonPath": 1028 passed, 8 skipped, 0 faileddotnet test src/embed_tests/ --filter "FullyQualifiedName~SetPythonPath": 1 passedpytest --runtime netcore tests: 478 passed, 23 skippeddotnet test src/python_tests_runner/: 2 passedinitializeeach made their first overloaded call with aDecimalargument and readself.time/self.utc_time. All returned the expected values, and the backtest completed and traded normally.🤖 Generated with Claude Code