orocos / orocos_kinematics_dynamics

Orocos Kinematics and Dynamics C++ library
672 stars 407 forks source link

Memory Leak in PyKDL Tree.getChain #211

Closed mag-sruehl closed 4 years ago

mag-sruehl commented 4 years ago

This code will consume will grow the memory usage on my pc from 107 mb at the first raw_input to 677 mb in the end:

import kdl_parser_py.urdf as urdf
import gc

robot = """<?xml version="1.0"?>
<robot name="visual">
  <link name="base_link"/>
  <link name="right_leg"/>
  <joint name="base_to_right_leg" type="fixed">
    <parent link="base_link"/>
    <child link="right_leg"/>
  </joint>
</robot>
"""
raw_input()

tree = urdf.treeFromString(robot)[1]
for i in range(1000000):
    tree.getChain("base_link", "right_leg")
gc.collect()
raw_input()

valgrind output (only the very last):

==14776== 545,924,946 (39,999,200 direct, 505,925,746 indirect) bytes in 999,980 blocks are definitely lost in loss record 712 of 712
==14776==    at 0x4C3017F: operator new(unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==14776==    by 0xB66143F: meth_Tree_getChain (in /opt/ros/melodic/lib/python2.7/dist-packages/PyKDL.so)
==14776==    by 0x1FCE69: call_function (ceval.c:4376)
==14776==    by 0x1FCE69: PyEval_EvalFrameEx (ceval.c:3013)
==14776==    by 0x1FA619: PyEval_EvalCodeEx (ceval.c:3608)
==14776==    by 0x1F9F38: PyEval_EvalCode (ceval.c:669)
==14776==    by 0x22B04E: run_mod.lto_priv.2104 (pythonrun.c:1385)
==14776==    by 0x226291: PyRun_FileExFlags (pythonrun.c:1371)
==14776==    by 0x225CBC: PyRun_SimpleFileExFlags (pythonrun.c:957)
==14776==    by 0x1D43E5: Py_Main (main.c:645)
==14776==    by 0x4E5DB96: (below main) (libc-start.c:310)

I used orocos_kinematics_dynamics 1.4.0 and current master. As this is similar to #157, I have built it with with the fixed 4.19-maint (v4.19.21 ??? January 2020) sip version.

Further, I suspect the problem is the new kinfam.sip:271

    Chain* getChain(const std::string& chain_root, const std::string& chain_tip)const;
%MethodCode
    Chain* chain = new Chain();
    sipCpp->getChain(*a0, *a1, *chain);
    sipRes = chain;
%End

But as I have no knowledge about sip, I am not sure about that. I will look further into it since I am desperate for a fix. Any help would be very welcome.

mag-sruehl commented 4 years ago

PR #212 will fix this issue.

MatthijsBurgh commented 4 years ago

I have been working on fixing the issues #41, #129, #157. These are all SIP related. These are fixed in SIP 4.19.21. So have you tried this with SIP 4.19.21?

mag-sruehl commented 4 years ago

Hi Matthijs! SIP was my main suspect for this memory leak, so build and test with 4.19.21 was my first move. But that did not fix it. Instead, the problem is in the sip declaration of Tree.getChain. There is a Chain instance new'ed in getChain which is returned by that function. Thus I assume the ownership of that object has to be transferred to python by declaring getChain as a "factory". That is what I propose in PR #212.

Disclaimer: All my python-sip knowlede is base on random findings form a one hour google session - I am clearly no expert on that topic.

MatthijsBurgh commented 4 years ago

212 is merged, but could you, @mag-sruehl, write a test, which fails in the old situation and succeeds in the new situation?

MatthijsBurgh commented 4 years ago

Please close this issue