Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

leaf systems implemented in python are immortal #22515

Open
rpoyner-tri opened this issue Jan 23, 2025 · 6 comments
Open

leaf systems implemented in python are immortal #22515

rpoyner-tri opened this issue Jan 23, 2025 · 6 comments
Assignees
Labels
component: pydrake Python API and its supporting Starlark macros type: bug

Comments

@rpoyner-tri
Copy link
Contributor

What happened?

First noticed in this stack overflow question: https://stackoverflow.com/questions/79380761/potential-memory-leaks-of-diagrams-with-leaf-systems-in-pydrake/

Confirmed in test cases here: https://github.com/rpoyner-tri/drake/releases/tag/py-leaf-leak

Apparently LeafSystem subclasses implemented in Python end up using/leaking bound methods in a way that can't be garbage collected.

Version

circa v1.37.0

What operating system are you using?

Ubuntu 22.04

What installation option are you using?

compiled from source code using Bazel

Relevant log output

rico@PUGET-255560:~/checkout/drake$ ./bazel-bin/bindings/pydrake/py/memory_leak_test TestMemoryLeaks.test_myadder
Regression test for memory leaks.

The test contains examples of pydrake code that may leak (DUTs),
instrumentation to detect leaks, and optional additional debug printing under
an internal verbose option.


sentinel: tracked myadder::adder 0x7c9301605e40
before collect
sentinel for myadder::adder
sentinel alive? True
ref_count: 4
is_tracked: True
generation: 0
referrers: [<bound method MyAdder.CalcSum of <memory_leak_test.MyAdder object at 0x7c9301605e40>>, <bound method MyAdder.CalcDifference of <memory_leak_test.MyAdder object at 0x7c9301605e40>>]
referents: [{'_a_port': <pydrake.systems.framework.InputPort object at 0x7c92fd949130>, '_b_port': <pydrake.systems.framework.InputPort object at 0x7c92fd991d70>}, <class 'memory_leak_test.MyAdder'>]
after collect
sentinel for myadder::adder
sentinel alive? True
ref_count: 4
is_tracked: True
generation: 2
referrers: [<bound method MyAdder.CalcSum of <memory_leak_test.MyAdder object at 0x7c9301605e40>>, <bound method MyAdder.CalcDifference of <memory_leak_test.MyAdder object at 0x7c9301605e40>>]
referents: [{'_a_port': <pydrake.systems.framework.InputPort object at 0x7c92fd949130>, '_b_port': <pydrake.systems.framework.InputPort object at 0x7c92fd991d70>}, <class 'memory_leak_test.MyAdder'>]
F
======================================================================
FAIL: test_myadder (memory_leak_test.TestMemoryLeaks)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/rico/checkout/drake/./bazel-bin/bindings/pydrake/py/memory_leak_test.runfiles/_main/bindings/pydrake/test/memory_leak_test.py", line 487, in test_myadder
    self.do_test(dut=_dut_myadder, count=1)
  File "/home/rico/checkout/drake/./bazel-bin/bindings/pydrake/py/memory_leak_test.runfiles/_main/bindings/pydrake/test/memory_leak_test.py", line 465, in do_test
    self.assertLessEqual(leaks, leaks_allowed)
AssertionError: 1 not less than or equal to 0

----------------------------------------------------------------------
Ran 1 test in 0.012s

FAILED (failures=1)
sentinel: unmade myadder::adder 0x7c9301605e40
@rpoyner-tri
Copy link
Contributor Author

Added a pretty minimal leak test at #22517. Here's the verbose output:

Executing tests from //bindings/pydrake:py/memory_leak_test
-----------------------------------------------------------------------------
Regression test for memory leaks.

The test contains examples of pydrake code that may leak (DUTs),
instrumentation to detect leaks, and optional additional debug printing under
an internal verbose option.



Running tests...
----------------------------------------------------------------------
sentinel: tracked mypyleafsystem::mypyleafsystem 0x71fe2e3a7940
before collect
sentinel for mypyleafsystem::mypyleafsystem
sentinel alive? True
ref_count: 1
is_tracked: True
generation: 0
referrers: []
referents: [{'_a_port': <pydrake.systems.framework.InputPort object at 0x71fe2e237cf0>}, <class 'memory_leak_test.MyPyLeafSystem'>]
after collect
sentinel for mypyleafsystem::mypyleafsystem
sentinel alive? True
ref_count: 1
is_tracked: True
generation: 2
referrers: []
referents: [{'_a_port': <pydrake.systems.framework.InputPort object at 0x71fe2e237cf0>}, <class 'memory_leak_test.MyPyLeafSystem'>]
.
----------------------------------------------------------------------
Ran 1 test in 0.014s

@jwnimmer-tri
Copy link
Collaborator

BTW I imagine the problem is the reference_internal marking on the return value of DeclareVectorInputPort (leading to a self-loop when assigned to a member field), but it's not obvious to me what the right marking would be.

@rpoyner-tri
Copy link
Contributor Author

Good call. I'll do some experiments.

@rpoyner-tri
Copy link
Contributor Author

In my experiment, ref_cycle<0,1> and py_rvp::reference (not _internal) do the trick. Now I have to find all of the similar bindings that need updating.

@rpoyner-tri
Copy link
Contributor Author

Hm, maybe the cycle is even unnecessary -- I'll need to think of a test case to decide that.

@rpoyner-tri
Copy link
Contributor Author

fixes are in development. Probably have to re-annotate all of Declare*Port and Get*Port.

@jwnimmer-tri jwnimmer-tri moved this from Todo to In Progress in #dynamics (Drake board) Jan 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: pydrake Python API and its supporting Starlark macros type: bug
Projects
Status: In Progress
Development

No branches or pull requests

2 participants