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

list of memory leaks #170

Closed
aymanhab opened this issue Aug 11, 2014 · 19 comments
Closed

list of memory leaks #170

aymanhab opened this issue Aug 11, 2014 · 19 comments
Assignees
Milestone

Comments

@aymanhab
Copy link
Member

The test case testInitState filas on Windows (2013 Pro) with error log below. Need to either plug leak if true, or relax condition if this's not true so that developers have confidence in their environment. The test case passes on Travis,
...
Loaded model arm26 from file arm26.osim
*********************** testMemoryUsage ***********************
MODEL: arm26.osim uses 136KB
156KB change in memory use after 100 state initializations.
Approximate leak size: 1.55957KB or 1.14674% of model size.
Average initialization time: 0.66ms
testInitState failed:
Exception:
testMemoryUsage: state initialization leak > 0.5% of model memory
footprint.
file= C:\Users\Ayman\Documents\GitHub\opensim-core\OpenSim\Simulation\Test\testInitState.cpp
line= 195

@aseth1
Copy link
Member

aseth1 commented Aug 11, 2014

We have no reason to think the leak is not legit. We can comb through https://gist.github.com/chrisdembia/8e2201c50d68f3b2f344 to find the main culprit, since @carmichaelong was seeing similar issues.
With 2013 Pro are you building 32bit?

@chrisdembia
Copy link
Member

I spent some time trying to fix that SimmSpline leak, but did not succeed.

@chrisdembia
Copy link
Member

There's also a leak in Joint's.

@carmichaelong
Copy link
Member

For my case, I've been building 64bit with 2013Pro.

@aymanhab
Copy link
Member Author

I'm building 32 bit using VisualStudio 2013 Pro (on 64 bit machines).

@aseth1
Copy link
Member

aseth1 commented Aug 11, 2014

which SimmSpline and Joint leak? It may be the case that it just barely passes in 64bit. It is worth patching known leaks. I can try to tackle the leaks @chrisdembia if you can point me to them.
Thanks!

@chrisdembia
Copy link
Member

@chrisdembia
Copy link
Member

More info: the new that causes the SimmSpline leak is here:

T **newArray = new T*[aCapacity];
.

Here is the sequence of calls:

==8317==    at 0x4C2AFE7: operator new[](unsigned long) (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==8317==    by 0x4F35085: OpenSim::ArrayPtrs<OpenSim::PropertyGroup>::ensureCapacity(int) (ArrayPtrs.h:396)
==8317==    by 0x4F32FD9: OpenSim::PropertySet::PropertySet() (ArrayPtrs.h:146)
==8317==    by 0x4EE7FBA: OpenSim::Object::Object() (Object.cpp:83)
==8317==    by 0x4EC0378: OpenSim::Function::Function() (Function.cpp:62)
==8317==    by 0x4F48551: OpenSim::SimmSpline::SimmSpline(int, double const*, double const*, std::string const&) (SimmSpline.cpp:75)
==8317==    by 0x5B18E41: OpenSim::Delp1990Muscle_Deprecated::setupProperties() (Delp1990Muscle_Deprecated.cpp:194)
==8317==    by 0x5B1ABA8: OpenSim::Delp1990Muscle_Deprecated::Delp1990Muscle_Deprecated(OpenSim::Delp1990Muscle_Deprecated const&) (Delp1990Muscle_Deprecated.cpp:112)
==8317==    by 0x5B1DEEE: OpenSim::Delp1990Muscle_Deprecated::clone() const (Delp1990Muscle_Deprecated.h:57)

@chrisdembia
Copy link
Member

@chrisdembia
Copy link
Member

Here's a leak in creating the AssemblySolver: https://github.com/opensim-org/opensim-core/blob/master/OpenSim/Simulation/Model/Model.cpp#L1542

The items in the coordsToTrack array are deleted but the array itself is not deleted: https://github.com/opensim-org/opensim-core/blob/master/OpenSim/Simulation/AssemblySolver.cpp#L76

@chrisdembia chrisdembia changed the title testInitState fails on windows testInitState fails on windows (list of memory leaks) Aug 13, 2014
@chrisdembia
Copy link
Member

I'm filing a separate issue for the assembly solver bug.

@aymanhab
Copy link
Member Author

@chrisdembia taking one leak at a time, does testInitState fail/leak due to SimmSpline? or should I make a commit with [ci valgrind] to rerun and comb thru output? Do you have a link to the latest valgrind output?

@chrisdembia
Copy link
Member

does testInitState fail/leak due to SimmSpline?

I think so, looking at the link below.

Do you have a link to the latest valgrind output?

it's only on my fork so far: https://travis-ci.org/chrisdembia/opensim-core/jobs/32566411#L3592

@chrisdembia
Copy link
Member

or should I make a commit with [ci valgrind] to rerun and comb thru output?

You should be able to use the link above for now.

@aymanhab
Copy link
Member Author

I can see it now, and I don't think t's the SimmSpline since this happens in the call stack of RegisterType which happens once per loading the dll. It is the AssemblySolver leak, I fixed it and the case passes on windows now but now testIK dies and I'm investigating. Thanks for the link @chrisdembia

@aymanhab aymanhab changed the title testInitState fails on windows (list of memory leaks) list of memory leaks Aug 22, 2014
@jenhicks
Copy link
Member

For tracking purposes, I think it would be easier to create separate issues. Otherwise this one might always be open ....

@aymanhab could you make appropriate issues for confirmed leaks and we can prioritize appropriately?

@aymanhab
Copy link
Member Author

@jenhicks I will list what was reported by valgrind.

@sherm1 sherm1 modified the milestone: Icebox Jan 8, 2015
@chrisdembia
Copy link
Member

The reason why we currently do not use travis to detect memory leaks is that we already have a bunch of leaks. However, it is possible to suppress specific errors with valgrind:

This would allow us to use valgrind to check for leaks on travis.

@aymanhab
Copy link
Member Author

With latest fixes verified by @fcanderson this can be put to rest finally

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants