swordlegend / recastnavigation

Automatically exported from code.google.com/p/recastnavigation
zlib License
0 stars 0 forks source link

Memory allocation issues #92

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
There is a possible quite serious bug in Recast's memory allocation. In the 
handleBuild() functions, you see a lot of code like that:

Code:

m_triflags = new unsigned char[m_ntris];
    if (!m_triflags)
    {

However, this code is not functional, since it doesn't conform to the C++ 
standard. Have a look a this link: 
http://msdn.microsoft.com/en-us/magazine/cc164087.aspx
I know that Recast doesn't use any STL containers, but in my own integration of 
Recast, "new" never returned NULL, even if it didn't manage to allocate the 
memory. Most of the time, this happens due to memory fragmentation.
As I understand the issue, "new" should throw a std::bad_alloc exception on all 
modern compilers. When people check for NULL, this usually means that they have 
used the VC++ 6.0 compiler for a long time and didn't notice that a central 
behavior has changed.
In my project, I changed the code to catch bad_alloc exceptions, and then I get 
the error I want. Most of the time, you just throw in too much geometry or you 
use too small voxels. Then, Recast has problems allocating the neccessary 
memory and its behavior is undefined. In my project, it kept crashing until I 
fixed the allocation checking code.

I never really had the time to tell Mikko this, although I wanted for a long 
time, sorry there!
Here's another link for that problem:
http://support.microsoft.com/?scid=k...67733&x=11&y=8

A possible solution could look like this:

Code:

#include <stdext>
try
{
  handleBuild()
}
catch (std::bad_alloc&)
{
   // write some error message
}

I have to say that I'm not 100% sure about this, but it looks this way.
Just try that, and you will see.

I think this is an issue, because Recast's code is a mixture of C and C++. You 
really have to decide on which language you want to use for Recast. If you want 
to use classes and "new", please also adopt the exception style.
However, as most of Recast's code is already in C-style, it probably would be 
better to move to C completely...

There is also a compiler switch to deactivate C++ exceptions, but please 
*don't* use it! Recast will be included in many applications, and many are 
written in C++ with exceptions. Mixing these settings is probably not possible 
at all.

Original issue reported on code.google.com by j.d...@gmx.de on 1 Jul 2010 at 9:01

GoogleCodeExporter commented 9 years ago
Most console games ship with exceptions disabled.

Original comment by cameron....@gmail.com on 8 Jul 2010 at 11:36

GoogleCodeExporter commented 9 years ago
I'm marking this as duplicate since this will get fixed when I finish the work 
on custom memory allocators (issue 78). That will make the memory management 
more malloc style.

My intention is that Recast (and Detour) can be used in console game type 
scenarios where you don't usually have exceptions, I also wish that Recast will 
die nicely when it runs out of memory when used with a tool on desktop.

Original comment by memono...@gmail.com on 8 Jul 2010 at 2:56