new and delete, pointer arrays not working with static variable

[Swoorup]

I am using the following code.

void CPathFind::AllocatePathFindInfoMem(void){
    if (g_pCarPathInfos){
        delete [] g_pCarPathInfos;
        g_pCarPathInfos = NULL;
    }
    if (g_pPedPathInfos){
        delete [] g_pPedPathInfos;
        g_pPedPathInfos = NULL;
    }
    if (g_pTempDetachedNodes){
        delete [] g_pTempDetachedNodes;
        g_pTempDetachedNodes = NULL;
    }


    g_pCarPathInfos = new CPathInfoForObject[12288];
//    memset(g_pCarPathInfos, 0, sizeof(CPathInfoForObject) * 12288);
    g_pPedPathInfos = new CPathInfoForObject[14568];
    memset(g_pCarPathInfos, 0, sizeof(CPathInfoForObject) * 14568);
    g_pTempDetachedNodes = new CTempDetachedNode[4600];
    memset(g_pTempDetachedNodes, 0, sizeof(CTempDetachedNode) * 4600);
    g_nPedGroupNodes = 0;
    g_nCarGroupNodes = 0;
}


void CPathFind::PreparePathData(void){
    //CDebug::DebugAddText("Custom Code: PreparePathData");
    if ( g_pCarPathInfos && g_pPedPathInfos && g_pTempDetachedNodes){
        CTempNode* pTempNodes = new CTempNode[5000];
        m_nAttachedPoints = 0;
        m_nAttachedNodes = 0;
        PreparePathDataForType(PATHDATAFOR_CAR, pTempNodes, NULL, 1.0f, g_pCarPathInfos, g_nCarGroupNodes);
        m_nCarAttachedNodes = m_nAttachedNodes;
        PreparePathDataForType(PATHDATAFOR_PED, pTempNodes, NULL, 1.0f, g_pPedPathInfos, g_nPedGroupNodes);
        m_nPedAttachedNodes = m_nAttachedNodes - m_nCarAttachedNodes;
        if (pTempNodes)
            delete [] pTempNodes;
        CountFloodFillGroups(PATHDATAFOR_CAR);
        CountFloodFillGroups(PATHDATAFOR_PED);
        delete [] g_pCarPathInfos;
        g_pCarPathInfos = NULL;
        delete [] g_pPedPathInfos;
        g_pPedPathInfos = NULL;
        if (g_pTempDetachedNodes)
            delete [] g_pTempDetachedNodes;
        g_pTempDetachedNodes = NULL;
    }

The 2 functions belong to a class named CPathFind. The variable whose prefixes begin with g_ are the static elements of the class.

At first the AllocatePathFindMemInfo is called and then at last the PreparePathData is called. And the error is encountered at this line :
delete [] g_pCarPathInfos;

MSVC then gives me this error

Windows has triggered a breakpoint in NewPathFind.exe.


This may be due to a corruption of the heap, which indicates a bug in NewPathFind.exe or any of the DLLs it has loaded.


This may also be due to the user pressing F12 while NewPathFind.exe has focus.


The output window may have more diagnostic information.

Help!

[laserlight]

Why are you not using std::vector or some other container instead of manual memory management? Are you absolutely sure that using static member variables here is the right approach?

[Swoorup]

Yes, I am pretty sure, since there are large number of dependencies of this specific class and its static variable. What could be the problem,?

[laserlight]

Yes, I am pretty sure, since there are large number of dependencies of this specific class and its static variable.

That sounds like circular reasoning. If you want to justify the use of a static non-const member variable, it could be because it makes sense to have state that is shared by all objects of the class, not because "there are large number of dependencies of this specific class and its static variable". Are the static member variables private?

What could be the problem,?

Could be a double deletion.

I would re-look into your class design and change the static member variables into non-static member variables if feasible. Instead of using new[] and delete[] directly, I would use std::vector or some other appropriate container.

[Salem]

Well the kludge of trying to clear memory needs to be fixed.

g_pCarPathInfos = new CPathInfoForObject[12288];
memset(g_pCarPathInfos, 0, sizeof(CPathInfoForObject) * 14568);

That's a lot of memory you're trashing.

Now, is CPathInfoForObject just a struct containing ONLY simple data like char and int (NOT pointers, doubles or C++ objects like std::string)?
If it isn't, then you're doing the wrong thing.

Actually, scrub that - you're doing the wrong thing PERIOD.
Just make a constructor for your CPathInfoForObject (yes, even if it's just a C-style struct, it can have a constructor) and be done with it. new will arrange for your ctor to be called when allocating the memory to begin with.

Next, replace some magic numbers with say
const int defaultMaxCarPaths = 12288;

> Yes, I am pretty sure, since there are large number of dependencies of this specific class and its static variable.
Your classes should be defined entirely by a public function interface. Nobody should care whether you're using a static array or a std::vector (or anything else).
Data Encapsulation in C++

[Swoorup]

The static variable is public, and is accessed directly by other functions using CPathFind::g_pPedPathInfos. It seems that the delete operator is automatically invoked somewhere

[laserlight]

The static variable is public, and is accessed directly by other functions using CPathFind::g_pPedPathInfos.

Therefore, you are using global variables, and that is part of the problem.

It seems that the delete operator is automatically invoked somewhere

Since you are not using containers or smart pointers, it wouldn't be automatic. More likely, you used delete[] elsewhere when you shouldn't, but because of the use of global variables, you find it hard to determine where.

[Swoorup]

Nop, I haven't really used anywhere else other than those two function.

@Salem,
the CPathInfoForObject just simply contains simple data like char, int, short,etc.
(Next, replace some magic numbers with say
const int defaultMaxCarPaths = 12288; )
Thanks, I will take note of this.

CPathInfoForObject

struct CPathInfoForObject{
  float fX; 
  float fY; 
  float fZ; 
  uint8_t byteNodeType;
  signed char sbNextNode;
  unsigned char byteLeftLanes;
  unsigned char byteRightLanes;
  unsigned char byteSpeedLimit;
  signed char sbMedianWidth;
  unsigned char byteFlags;
  unsigned char byteSpawnRate;
};

EDIT: Same error happens even when using malloc and free

[Swoorup]

Sorry about the double post.
The error was due to limited heap size when actually greater array was used!