Memory Leak in ColladaConverter with MeshInterface

[Digitalghost]

A btTriangleMesh is created to support a btBvhTriangleMeshShape in ColladaConverter. Because ColladaConverter does this "new" itself, it is required to delete the btTriangleMesh. ColladaConverter doesn't delete the triangle mesh, the memory leaks.

I don't think that the ColladaConverter should be responsible to delete the triangle mesh though, because it requires a lot of "if dynamic_cast<btTriangleMeshShape*>(shape)" to check if the shape is a triangle mesh shape and then deleting the underlying triangle mesh and the shape together. This gets ugly.

I think a better solution would be to pass in a flag that allows the triangle mesh shape to own the mesh interface. This flag would ensure that the mesh interface gets deleted when the shape gets deleted.

So basically this line in ColladaConverter.cpp:

Code: Select all

rbOutput.m_colShape = new btBvhTriangleMeshShape(trimesh,useQuantizedAabbCompression);

Would look something like this

Code: Select all

rbOutput.m_colShape = new btBvhTriangleMeshShape(trimesh,useQuantizedAabbCompression,true);

Where the "true" means that the shape owns the "trimesh" object, and it will perform its own cleanup.

What do you think?

[Digitalghost]

Also, in daeErrorHandler.cpp, this code:

Code: Select all

daeErrorHandler::~daeErrorHandler() {
	if (_instance != NULL && _default ) {
		delete _instance;
		_instance = 0;
	}
}

doesn't make sense and causes an infinite loop. It should be this:

Code: Select all

daeErrorHandler::~daeErrorHandler() {
_instance = 0;
}

The instance is already being deleted, so recursively calling delete causes a stack overflow.

[Erwin Coumans]

Bullet maintains the policy: who news/allocates/creates is responsible for delete/deallocate/destruction. So we can't implement your suggestion, it violates this basic policy.

A btTriangleMesh is created to support a btBvhTriangleMeshShape in ColladaConverter. Because ColladaConverter does this "new" itself, it is required to delete the btTriangleMesh. ColladaConverter doesn't delete the triangle mesh, the memory leaks.

We will fix the memory leak in the ColladaConverter by keeping an array of allocated data (including btTriangleMesh) so it can be destroyed later. It is on the todo list for upcoming 2.72 release: http://code.google.com/p/bullet/issues/detail?id=92

Also, in daeErrorHandler.cpp, this code:

This is not related to Bullet, please report at the COLLADA forums. We will upgrade to a newer version of COLLADA DOM at some stage, perhaps it has been fixed already.

Thanks for the feedback,
Erwin