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

Memory leaks in reading meshes using Assimp #565

Open
jslee02 opened this issue Dec 6, 2015 · 3 comments
Open

Memory leaks in reading meshes using Assimp #565

jslee02 opened this issue Dec 6, 2015 · 3 comments

Comments

@jslee02
Copy link
Member

jslee02 commented Dec 6, 2015

Memory leaks in reading meshes was reported by @sehoonha. Here is the test code:

TEST(SdfParser, SdfParserMemoryLeak)
{
  // Regression test
  for (int i = 0; i < 18; i++)
  {
    SkeletonPtr temp = SoftSdfParser::readSkeleton(
          DART_DATA_PATH"sdf/atlas/atlas_v3_no_head_soft_feet.sdf");
    if (!temp)
    {
      std::cout << "Load " << i << "-th atlas NG" << std::endl;
      exit(0);
    } else
    {
      std::cout << "Load " << i << "-th atlas OK" << std::endl;
    }
  }
}

Once it reached the 18-th iteration, it fails to allocate memory.

The main cause was that LocalResource instance is not released after it's created. Simply, AssimpInputResourceRetrieverAdaptor::Open() is called when Assimp opens a file but Assimp doesn't call AssimpInputResourceRetrieverAdaptor::Close() when the file needs to be closed. Instead, it just destructs the AssimpInputResourceRetrieverAdaptor instance.

This is reported to the upstream.

I quickly tested with my patch, that is described in the report, for two mesh types (stl, dae), and it worked. But I would like to wait for feedbacks from the community before create a pull request to see if it's the correct approach. It seems this patch should be applied to other mesh types (more than 40!) so I don't want to start it without assurance.

@kimkulling
Copy link

Is this issue only related to Collada files? Because in assimp-master I just fixed this leak.

Kimmi

@jslee02
Copy link
Member Author

jslee02 commented Dec 6, 2015

Thank you for the real quick fix!

I observed that it also happens in STLImporter::InternReadFile(). Other importers are not tested, but I think it would probably happen in other importers that call IOSystem::Open() but don't call `IOSystem::Close() after file import.

Here is the search result of the calls of IOSystem::Open() and IOSystem::Close():

js@js:~/dev/assimp$ grep -rnw '.' -e "pIOHandler->Open"
./doc/dox.h:1683:   boost::scoped_ptr<IOStream> file( pIOHandler->Open( pFile, "rb"));
./code/3DSLoader.cpp:158:    StreamReaderLE stream(pIOHandler->Open(pFile,"rb"));
./code/BaseImporter.cpp:152:    boost::scoped_ptr<IOStream> pStream (pIOHandler->Open(pFile));
./code/BaseImporter.cpp:259:    boost::scoped_ptr<IOStream> pStream (pIOHandler->Open(pFile));
./code/COBLoader.cpp:147:    boost::scoped_ptr<StreamReaderLE> stream(new StreamReaderLE( pIOHandler->Open(pFile,"rb")) );
./code/RawLoader.cpp:102:    boost::scoped_ptr<IOStream> file( pIOHandler->Open( pFile, "rb"));
./code/BlenderLoader.cpp:166:    boost::shared_ptr<IOStream> stream(pIOHandler->Open(pFile,"rb"));
./code/DXFLoader.cpp:137:    boost::shared_ptr<IOStream> file = boost::shared_ptr<IOStream>( pIOHandler->Open( pFile) );
./code/TerragenLoader.cpp:123:    IOStream* file = pIOHandler->Open( pFile, "rb");
./code/OgreMaterial.cpp:172:            materialFile = pIOHandler->Open(potentialFiles[i]);
./code/glTFImporter.cpp:105:        boost::scoped_ptr<IOStream> pStream(pIOHandler->Open(pFile));
./code/Q3DLoader.cpp:110:    StreamReaderLE stream(pIOHandler->Open(pFile,"rb"));
./code/ASELoader.cpp:133:    boost::scoped_ptr<IOStream> file( pIOHandler->Open( pFile, "rb"));
./code/HMPLoader.cpp:117:    boost::scoped_ptr<IOStream> file( pIOHandler->Open( pFile));
./code/UnrealLoader.cpp:158:    IOStream* p = pIOHandler->Open(d_path);
./code/UnrealLoader.cpp:161:    StreamReaderLE d_reader(pIOHandler->Open(d_path));
./code/UnrealLoader.cpp:206:    p = pIOHandler->Open(a_path);
./code/UnrealLoader.cpp:209:    StreamReaderLE a_reader(pIOHandler->Open(a_path));
./code/UnrealLoader.cpp:238:    boost::scoped_ptr<IOStream> pb (pIOHandler->Open(uc_path));
./code/MS3DLoader.cpp:216:    StreamReaderLE stream(pIOHandler->Open(pFile,"rb"));
./code/MD5Loader.cpp:357:    boost::scoped_ptr<IOStream> file( pIOHandler->Open( pFile, "rb"));
./code/MD5Loader.cpp:572:    boost::scoped_ptr<IOStream> file( pIOHandler->Open( pFile, "rb"));
./code/MD5Loader.cpp:684:    boost::scoped_ptr<IOStream> file( pIOHandler->Open( pFile, "rb"));
./code/FBXImporter.cpp:144:    boost::scoped_ptr<IOStream> stream(pIOHandler->Open(pFile,"rb"));
./code/MD3Loader.cpp:740:    boost::scoped_ptr<IOStream> file( pIOHandler->Open( pFile));
./code/MD2Loader.cpp:197:    boost::scoped_ptr<IOStream> file( pIOHandler->Open( pFile));
./code/OFFLoader.cpp:112:    boost::scoped_ptr<IOStream> file( pIOHandler->Open( pFile, "rb"));
./code/STLLoader.cpp:175:    boost::scoped_ptr<IOStream> file( pIOHandler->Open( pFile, "rb"));
./code/NDOLoader.cpp:117:    StreamReaderBE reader(pIOHandler->Open( pFile, "rb"));
./code/LWSLoader.cpp:514:    boost::scoped_ptr<IOStream> file( pIOHandler->Open( pFile, "rb"));
./code/ColladaParser.cpp:91:    boost::scoped_ptr<IOStream> file( pIOHandler->Open( pFile ) );
./code/BVHLoader.cpp:121:    boost::scoped_ptr<IOStream> file( pIOHandler->Open( pFile));
./code/XFileImporter.cpp:114:    boost::scoped_ptr<IOStream> file( pIOHandler->Open( pFile));
./code/SMDLoader.cpp:125:    boost::scoped_ptr<IOStream> file( pIOHandler->Open( pFile, "rb"));
./code/NFFLoader.cpp:136:    boost::scoped_ptr<IOStream> file( pIOHandler->Open( path, "rb"));
./code/NFFLoader.cpp:238:    boost::scoped_ptr<IOStream> file( pIOHandler->Open( pFile, "rb"));
./code/irrXMLWrapper.h:59: * boost::scoped_ptr<IOStream> file( pIOHandler->Open( pFile));
./code/MDCLoader.cpp:222:    boost::scoped_ptr<IOStream> file( pIOHandler->Open( pFile));
./code/OgreXmlSerializer.cpp:745:    boost::scoped_ptr<IOStream> file(pIOHandler->Open(filename));
./code/IRRLoader.cpp:905:    boost::scoped_ptr<IOStream> file( pIOHandler->Open( pFile));
./code/AssbinLoader.cpp:87:    IOStream * in = pIOHandler->Open(pFile);
./code/AssbinLoader.cpp:639:    IOStream * stream = pIOHandler->Open(pFile,"rb");
./code/OgreImporter.cpp:101:    IOStream *f = pIOHandler->Open(pFile, "rb");
./code/OgreBinarySerializer.cpp:905:    IOStream *f = pIOHandler->Open(filename, "rb");
./code/OpenGEXImporter.cpp:248:    IOStream *file = pIOHandler->Open( filename, "rb" );
./code/MDLLoader.cpp:158:    boost::scoped_ptr<IOStream> file( pIOHandler->Open( pFile));
./code/IRRMeshLoader.cpp:124:    boost::scoped_ptr<IOStream> file( pIOHandler->Open( pFile));
./code/LWOLoader.cpp:142:    boost::scoped_ptr<IOStream> file( pIOHandler->Open( pFile, "rb"));
./code/XGLLoader.cpp:156:    boost::shared_ptr<IOStream> stream( pIOHandler->Open( pFile, "rb"));
./code/CSMLoader.cpp:125:    boost::scoped_ptr<IOStream> file( pIOHandler->Open( pFile, "rb"));
./code/ObjFileImporter.cpp:121:    boost::scoped_ptr<IOStream> file( pIOHandler->Open( pFile, mode));
./code/ACLoader.cpp:790:    boost::scoped_ptr<IOStream> file( pIOHandler->Open( pFile, "rb"));
./code/PlyLoader.cpp:130:    boost::scoped_ptr<IOStream> file( pIOHandler->Open( pFile));
./code/B3DImporter.cpp:116:    boost::scoped_ptr<IOStream> file( pIOHandler->Open( pFile));
./code/C4DImporter.cpp:134:    boost::scoped_ptr<IOStream> file( pIOHandler->Open( pFile));
./code/MDLMaterialLoader.cpp:66:    IOStream* pcStream = pIOHandler->Open(configPalette,"rb");
./code/IFCLoader.cpp:171:    boost::shared_ptr<IOStream> stream(pIOHandler->Open(pFile));

js@js:~/dev/assimp$ grep -rnw '.' -e "pIOHandler->Close"
./code/ColladaParser.cpp:107:    pIOHandler->Close( file.get() );
./code/AssbinLoader.cpp:94:    pIOHandler->Close(in);
./code/AssbinLoader.cpp:684:    pIOHandler->Close(stream);

I can't say the searched numbers of pIOHandler->Open and pIOHandler->Close should be equal, but I think it would worth to check.

@stale
Copy link

stale bot commented Feb 13, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Feb 13, 2018
@stale stale bot removed the stale label Feb 13, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants