Huge Memory Leak in GrassLoader (and Solution)

Since I implemented Grass Painting in Ogitor, i realized that there was a huge memory leak causing my app to use over 1GB memory and increased forever till it crashes due to insufficient memory problems.

So i took a look at what was wrong with GrassLoader. At first it seemed alright since loadPage() was creating meshes and entites and unloadPage() was destroying them. So what was wrong???

Finally i found out why: GRASS PAGES ARE NEVER UNLOADED!!!

The reason is in a small detail:

PagedGeometry.h
virtual void unloadPage(const PageInfo &page) {}

GrassLoader.h
/** INTERNAL FUNCTION - DO NOT USE */
void unloadPage(PageInfo &page);

do you see the problem??? The virtual function defines the variable as const, but the function which tries to OVERRIDE it is non-const, so the compiler thinks they are different functions and all the virtual calls are directed to the first empty definition. GrassLoader::unloadPage() is never called.

Solution is Simple, just edit PagedGeometry.h and replace:

virtual void unloadPage(const PageInfo &page) {}

with

virtual void unloadPage(PageInfo &page) {}

because grassloader needs the parameter non-const. This solves the problem and now compiler correctly overrides the base function. Voila, now pages really unload and no memory leaks...

Problem is deeper than i thought. The meshList is a member of PageInfo, but pageInfo is not a persistent struct, so the meshList is never kept. Working on it, will post a solution soon..

Final Solution is below as an attachment (No more Leaks):

[attachment=0]pg.rar[/attachment]

Awesome Stealth!

Can you submit a patch to the tracker?

-Fish

Unfortunately i am not using pagedgeometry's SVN, we have a mercurial repository, but the changes are below:


=== (+3,-1) Dependencies/PagedGeometry/include/PagedGeometry.h ===
@@ -1084,6 +1084,8 @@

bool mHasQueryFlag;
Ogre::uint32 mQueryFlag;
+
+ std::vector<Ogre::Mesh*> meshList;
};


@@ -1251,7 +1253,7 @@
In most cases you won't need to implement this function in your page loader at all,
since addEntity() is usually all that is used.
*/
- virtual void unloadPage(const PageInfo &page) {}
+ virtual void unloadPage(PageInfo &page) {}

/**
\brief Provides a method for you to perform per-frame tasks for your PageLoader if overridden (optional)



=== (+2,-0) Dependencies/PagedGeometry/source/PagedGeometry.cpp ===
@@ -862,6 +862,7 @@
mainGeom->getPageLoader()->loadPage(info);

page->_userData = info.userData;
+ page->meshList = info.meshList;

page->build();
page->setVisible(page->_visible);
@@ -887,6 +888,7 @@
info.xIndex = page->_xIndex;
info.zIndex = page->_zIndex;
info.userData = page->_userData;
+ info.meshList = page->meshList;

//Unload the page
page->removeEntities();


I hope you can create the patch using this data...

Appreciated, stealth.
I am also working on grass paint,using MFC as GUI.With previous PG code,werid runtime error appeared.Now You saved me~~

as far as i see this is a duplicate to a bug report i fixed earlier today.

instead destroying a page in "unloadPage" it removes its meshes in the page's destructor.

Please try to update to the latest SVN version and test and report back if the problem persists.

thank you!

tdev:

Since currently i am on a trip, i couldnt check your latest changes, but;

If you destroy meshes in destructor, it still would not be convenient. Since if we modify the density map and call reloadgeometrypages(...), the old meshes will still remain in memory, think that we are calling it 10+ times a second, it grows up to GByte limits and crashes the application.... So, the meshes must be either destroyed on unloadPage() or at least destroyed during a reloadgeometry|pages() call (I know later would be better since unloading and loading is already done duing paging and may cause performance loss)

tdev:
If you destroy meshes in destructor, it still would not be convenient. Since if we modify the density map and call reloadgeometrypages(...), the old meshes will still remain in memory, think that we are calling it 10+ times a second, it grows up to GByte limits and crashes the application.... So, the meshes must be either destroyed on unloadPage() or at least destroyed during a reloadgeometry|pages() call (I know later would be better since unloading and loading is already done duing paging and may cause performance loss)
thanks for your feedback, i have also experienced this problem and re-added the code in unloadpage. I guess we need to verify the memory usage for this or even better: start coding on a plugin for 1.7 paging