memory leak

[gjaegy]

Hi,

I am just tracking memory leaks in my application, and I found one in Bulltet:

Code: Select all

~ConvexResult(void)
{
	delete mHullVertices;
	delete mHullIndices;
}

should be:

Code: Select all

~ConvexResult(void)
{
	delete[] mHullVertices;
	delete[] mHullIndices;
}

as memory is allocated using new xxxx[]

Also, I have memory leak destroy the btCompoundShape. It seems that when you delete such an object its children are not destroyed. Is that an intended behaviour ?

Thanks !

[Erwin Coumans]

I am just tracking memory leaks in my application, and I found one in Bulltet:

Code: Select all

~ConvexResult(void)
{
	delete mHullVertices;
	delete mHullIndices;
}

should be:

Code: Select all

~ConvexResult(void)
{
	delete[] mHullVertices;
	delete[] mHullIndices;
}

as memory is allocated using new xxxx[]

Thanks for the report, this code, located in Bullet/Extras/ConvexDecomposition/ConvexDecomposition.h, will be fixed.

Also, I have memory leak destroy the btCompoundShape. It seems that when you delete such an object its children are not destroyed. Is that an intended behaviour ?

Bullet uses the rule that the one who creates/allocates memory is responsible for freeing/deleting it. Since the btCompoundShape doesn't create its own children, the user is responsible to keep track of collision shapes. Keep in mind it is best to share collision shapes as much as possible.

Thanks for the feedback, hope this helps,
Erwin

[gjaegy]

All right, this makes sense, I will remember it.

BTW, I asked two questions some time ago and haven't get any feedback since:

- Shouldn't the btSoftRigidDynamicsWorld::debugDrawWorld() be public (as base class) ?

- I forward declare all the bullet types in one of my headers, and include the Bullet headers in my .cpp files, in order not to have any dependencies from Bullet in my other projects. However, I have an issue with btSoftBody::btSoftBodyWorldInfo and it is impossible to forward declare a struct nested in a class. Would it be possible to pull this struct out of the btSoftBody class ?

If you have some time to answer them, I would really appreciate !!

Thanks !

[gjaegy]

There is also a leak in ConvexBuilder.cpp, line 152, you should replace:

Code: Select all

	//don't do anything if hull is empty
	if (!tcount)
		return 0;

by

Code: Select all

	//don't do anything if hull is empty
	if (!tcount)
	{
		Vl_releaseVertexLookup(vc);
		return 0;
	}

as "vc" is created some lines above this through Vl_createVertexLookup().



CreateConvexHull() is called some times allocating some memory to the result object without matching call to ReleaseResult(). Will fix it tomorrow and tell you where exactly.

[Wavesonics]

gjaegy you have done a great service by finding these! I'm using Bullet in a commercial product running on what is essentially a embedded system so tracking memory leaks for my project is very difficult.

Hope this fix is in and a new stable release of Bullet comes out with it before I have to release

[gjaegy]

I am using bullet in a commercial product as well, so this is quite normal. Actually by tracking leak in my software I track Bullet's one in the same time

Glad I can help a (very) little bit this great library !!

It seems there are some others, so I will spend my day tracking those again. Will let you know.

Concerning ReleaseResult():
- one should be addded in ConvexBuilder.cpp line 186
- one should be addded in ConvexBuilder.cpp line 329 after "mCallback->ConvexDecompResult(r);"
- one in ConvexDecomposition.cpp line 164 after "callback->ConvexDecompResult(r);"

[edit] I couldn't find any other memory leak in the part of the library I use.

[edit 2] Yes there are others:
in btSoftBody.cpp, line 765, mCluster are allocated but never freed (m_clusters = new(btAlignedAlloc(sizeof(Cluster),16)) Cluster();)
You could add this in class destructor:

for(i=0;i<m_clusters.size();++i)
btAlignedFree(m_clusters);

[gjaegy]

There are some memory leaks left in the btSoftBody class (using clusters) that I can't get. Could someone have a look ?

[gjaegy]

Nobody ?

Doesn't anybody care about memory leaks ?! Personally I prefer a stable library than an unstable one with loads of features... Just my opinion though (maybe it is because our products need to run for days without being turned off. so even if running on Vista and using the low fragmentation heap, I am very concerned about mem allocations - leaks are evil for me).

[abaraba]

hi there gjaegy,

i see you're passionate about memory leaks...

anyway,
i never thought i could program a "memory leak" while in a wake state, so i never cared to try and track any and so never used any of these tools and so on... apparently im very wrong and coincidentally we are having a conversation about memory leaks over in the neighbouring thread, i thought you wouldnt mind sharing a few lessons?

http://www.bulletphysics.com/Bullet/php ... =17&t=2277

basically,
i ask whats happening in this example, is it bad and why:

Code: Select all

box1 = new btRigidBody( btScalar(0.0), (new btDefaultMotionState(trans)), 
     (new btBoxShape(btVector3(20,20,20))), btVector3(0,0,0));

  dynamicsWorld->addRigidBody(box1);

box2 = new btRigidBody( btScalar(1.0), (new btDefaultMotionState(trans)), 
     (new btBoxShape(btVector3(5,5,5))), btVector3(1,1,1));

  dynamicsWorld->addRigidBody(box2);

cheers

[gjaegy]

Hi abaraba,

tracking memory leaks is not my favourite task it is actually a real painful and annoying activity, but required to ensure the system performance don't degrade with the time...

using Visual Studio makes it very easy to find out leaks. simply add the following line at the start of the program and run in debug mode:

Code: Select all

_CrtSetDbgFlag ( _CRTDBG_ALLOC_MEM_DF | _CRTDBG_LEAK_CHECK_DF ); 

From what Erwin said, the one that creates the object should also free it. you create 2 motion states and two box shapes without storing them somewhere, so it is likely that you don't free them.

[abaraba]

thanks,

i dont use VS, but Linux and basic text editor, so i can't do that conveniently

i thought with your great knowledge about Bullet constructors/destructors and general new/delete functionality you would shed little bit more lite on whats really happening inside such a call, when and by whom is whose constructor being called..

but,
most basic question about that construct - is it a "memory leak" or not? ...my theory is that it is not, i believe these object created by *new*, inside the argument list are truly temporary, i would think that very next line the handle to that memory is lost and free for use.. would you know about that?

id be happy to look it up on google or wikipedia.. or where else, but what to search for?


cheers

[gjaegy]

no, object are not created temporary, and this need to be destroyed explicitely.

this is because you create them using new().

they would be temporary if the fonction would have accepted arguments passed by reference. for instance:

Code: Select all

void SetValue(SVector3D& _vRef)
{
   m_fX = _vRef.m_fX;
   ...
}

SVector3D v;
v.SetValue(SVector3D(0.0f, 0.0f, 0.0f));

in this example a temporary SVector3D(0.0f, 0.0f, 0.0f) is created, and will be destroyed automatically once it goes out of scope. This is not the case for a pointer, you have to manually free it (as it is not destroyed automatically when the variable storing it goes out of scope).

I hope this helps. Sorry I can't elaborate I am a bit busy...

[Erwin Coumans]

Thanks for the discussion and bringing up those leaks. Can you please report those bugfixes in the issue tracker, so they won't get lost?

http://code.google.com/p/bullet/issues/list

Thanks,
Erwin

[abaraba]

thanks, thats plenty...

when using new() - delete your pointers!
...and so, be sure to actually have pointers, not like that terrible example above


cheers

[gjaegy]

Erwin, are the fixes included in the latest 2.71 alpha release ?