GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Fix MethodBinding/OverloadMapper memory leak by Martin-Molinero · Pull Request #117 · QuantConnect/pythonnet

Conversation

@Martin-Molinero

@Martin-Molinero Martin-Molinero commented Jun 18, 2026

Copy link
Copy Markdown
Member

Cherry-pick from upstream pythonnet/pythonnet: fixes a long-standing memory leak in MethodBinding/OverloadMapper where references to the target/target-type were not owned/disposed.

  • Upstream PR: Fix MethodBinding/OverloadMapper memory leak (#691) pythonnet/pythonnet#2719 ("Fix MethodBinding/OverloadMapper memory leak", fixes long-standing issue Memory Leak on MethodBinding for generic method pythonnet/pythonnet#691), by @greateggsgreg
  • Upstream commit: pythonnet/pythonnet@ca323cc1
  • Cherry-picked with -x (original authorship preserved).

Conflict note: small conflicts in MethodObject.cs, OverloadMapper.cs, and test_method.py were resolved against this fork. The fork's ClassManager.GetClass(t) is => ReflectedClrType.GetOrCreate(t), so the upstream form (ReflectedClrType.GetOrCreate(...).NewReference()) is equivalent here; the leak fix adds owned references that are disposed in OnClear. Builds clean (Python.Runtime.csproj). Worth a careful review of the reference-ownership given the fork's divergence.

Relevant to long-running embedded use (e.g. Lean).

🤖 Generated with Claude Code

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@greateggsgreg @filmor @Martin-Molinero
Fix MethodBinding/OverloadMapper memory leak (pythonnet#691) (pythonn…
a346bc6
…et#2719)

* Fix MethodBinding/OverloadMapper memory leak (pythonnet#691)

MethodBinding and OverloadMapper held PyObject `target` references that
were not disposed during tp_clear, leaving Python-side refcount drops to
wait on the multi-hop .NET finalizer chain. They also shared the same
C# PyObject instance across mp_subscript/Overloads paths, so freeing one
could free the underlying Python object out from under the others.

- ExtensionType: add virtual OnClear() hook called from tp_clear before
  the GCHandle is released, letting subclasses eagerly drop owned
  Python references.
- MethodBinding/OverloadMapper: override OnClear to dispose `target`.
  (`targetType` is intentionally not disposed since Python types are
  long-lived and tracked by other caches.)
- Take an independent INCREF'd PyObject copy at every site that hands a
  shared target into a new MethodBinding or OverloadMapper, so each
  wrapper owns its own reference.

Result: the three _does_not_leak_memory tests drop from ~485 MB delta
to ~10 KB delta on Python 3.14.

* Tighten leak-test threshold to 10% to actually fail the bug

The previous 90% threshold (0.9 MB/iter against a 1 MB allocation)
documented the issue but did not reproduce it: master leaks
~600-765 KB/iter, which the 0.9 MB threshold accepts as passing.

Drop the threshold to 10% (104 KB/iter). On the 2026-05-09 verification
run with Python 3.14 GIL on linux-aarch64:

  Without fix (master):   ~572-765 KB/iter (FAIL)
  With fix (this branch): ~-500 B/iter     (PASS)

Margin is roughly 6x in either direction across .NET 8 and .NET 10, so
the threshold cleanly separates buggy from fixed states without being
sensitive to GC noise.

* Bugfix and improvements

- Handle the `PyType` reference in `OverloadMapper` and `MethodBinding` in the
  same way as the object reference
- Unconditionally store the `PyType` of the object
- Introduce `NewReference` helper function for the object and type
  passing
- Fix the remaining missing reference count bump for the type
  (`MethodObject`)
- As the count is now correct, `Dispose` the type as well

---------

Co-authored-by: Benedikt Reinartz <[email protected]>
(cherry picked from commit ca323cc)
jhonabreul
jhonabreul approved these changes Jul 7, 2026
View reviewed changes
@jhonabreul
jhonabreul merged commit 460abce into master Jul 7, 2026
4 checks passed

Uh oh!

There was an error while loading. Please reload this page.

@jhonabreul
jhonabreul deleted the cherry/methodbinding-overloadmapper-memleak branch July 7, 2026 20:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jhonabreul jhonabreul jhonabreul approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@Martin-Molinero @jhonabreul @greateggsgreg
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.