search

moveit / geometric_shapes

Representation of geometric shapes
57 stars 92 forks source link

Fix memory leak in createMeshFromBinary #191

Closed wxmerkt closed 3 years ago

wxmerkt commented 3 years ago

When using createMeshFromBinary scene gets created as a raw pointer but never deleted even after the pointer goes out of scope. This resulted in approximately 1.1 GB of memory leaked when loading some of our URDFs. Explicitly deleting the object after use fixes this problem.

tylerjw commented 3 years ago

You closed this, did you find this fix wasn't needed?

v4hn commented 3 years ago

The in-source documentation for Assimp::Importer::ReadFileFromMemory in Assimp 5.0.1 (also earlier versions) states:

 * The returned data is intended to be
 * read-only, the importer object keeps ownership of the data and will
 * destroy it upon destruction. [...]
 * The previous scene will be deleted during this call.

So this patch is invalid. If there is a memory leak here, it might be in Assimp itself.

wxmerkt commented 3 years ago

I closed the PR as the patch turned out to be invalid - there is a leak, however, this fix wouldn't be correct as AssimpImporter takes care of cleaning up its own memory. If I find a fix, I'll make a new PR.

v4hn commented 3 years ago

there is a leak [...] If I find a fix, I'll make a new PR.

Happy debugging! We're of course looking forward to the patch if the leak is in our code base.