Jump to content
  • Advertisement
Sign in to follow this  
silverphyre673

Huge memory leak issues.

This topic is 4795 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

I have a vector of a vector of pointers to a "TerrainCell" class as part of another "Mesh" class. This Mesh class basically holds the data for the terrain vertices and such for a chunk of a level. It has two major functions - InitializeMemory() and DeinitializeMemory(). It also has three main components - a width and height, and "mesh", which is: std::vector<std::vector<TerrainCell*> > mesh; I don't want to have to use up all the stack space. Anyways, "mesh" is supposed to have flat edges - the inner vectors should all have the same Size(). DeinitializeMemory() is supposed to delete all the pointers in mesh, and InitializeMemory() first calls DeinitializeMemory, then allocates new memory according to the width and height specified. Unfortunately, I made a simple memory-leak check, and it seems that the memory never gets deallocated. I was wondering how I am supposed to deallocate the memory in the vector successfully. Here is the relevant code: Mesh class declaration
//Declaration of TerrainCell (just a non-pointer array of 4 3d vertices)
#include "cell.h"

class Mesh
{
    public:
    Mesh();
    Mesh(unsigned int init_width, unsigned int init_height);
    ~Mesh();
    
    //Allocates memory for "mesh" according to "width" and "height"
    void InitializeMemory();
    //Deallocates all the memory in "mesh"
    void DeinitializeMemory();

    private:
    //Terrain vertices
    std::vector< std::vector<TerrainCell*> > mesh;
    //height: the number of outer vectors for "mesh"
    //width: the size of the inner vectors of "mesh"
    unsigned int height, width;
};


InitializeMemory()
void Mesh::InitializeMemory()
{
    DeinitializeMemory();
    unsigned int i = 0;
    for ( unsigned int out_count = 0; out_count < height; ++out_count )
    {
        std::vector<TerrainCell*> v_cells;
        for ( unsigned int in_count = 0; in_count < width; ++in_count )
        {
            TerrainCell * t_cell = new TerrainCell;
            v_cells.push_back(t_cell);
            ++i;
            //Thought the lack of "delete t_cell" might be the cause of the
            //memory leak.  Leaving it in causes a double free error.
            //delete t_cell;
        }
        mesh.push_back(v_cells);
    }
}


DeinitializeMemory()
void Mesh::DeinitializeMemory()
{
    //For each vector of vectors of TerrainCells
    for ( std::vector<std::vector<TerrainCell*> >::iterator iter_out = mesh.begin();
          iter_out != mesh.end();
          ++iter_out )
    {
        //Cycle through each cell in the vector
        for ( std::vector<TerrainCell*>::iterator iter_in = (*iter_out).begin();
              iter_in != (*iter_out).end();
              ++iter_in )
        {
            //And delete it... apparently unsuccessfully :(
            delete (*iter_in);
        }
        //Clear the row (neccessary?)
        (*iter_out).clear();
    }
    //Clear the mesh
    mesh.clear();
}


Obviously, in a terrain editor, this is a very big problem :) Thanks for any help you can provide. [Edited by - silverphyre673 on June 5, 2005 8:45:28 PM]

Share this post


Link to post
Share on other sites
Advertisement
I doubt there is actually that much memory at stake, to be honest. The vector holds onto the memory in case you want to use it again. If you will re-use the vector, then this probably isn't a problem. If you won't re-use the vector, just destroy it and again you should be ok.

But if you insist on keeping the vector but losing the memory, I believe that this function will work:
template < class T >
void shrinkCapacity(std::vector< T > & v) {
std::vector< T > tmp(v); // copy elements
v.swap(tmp); // then swap the refs
}


Call it on every vector you need 'shrinking'.

Share this post


Link to post
Share on other sites
Hmmm... I have been thinking, and I actually don't need the Mesh to be resizable. Thanks though. Basically the goal was to create a mesh of these TerrainCells that would represent one chunk of a level. If we moved to another chunk, then we would destroy the old chunk's data (which was originally loaded from a level info file) and then give it new memory. Thinking on it now, the way I had it was a very stupid way to do it, since each chunk would be the same size, and I was just going to fill in the data cell-by-cell as I loaded it.

Thanks, though!

Share this post


Link to post
Share on other sites
actually, I changed it, and it is now giving me "invalid pointer" errors at runtime on my linux box, where I am testing the memory leaks, and is, I believe, using up a bunch of system resources on the windows box because it gets progressively slower. Here is the revised code (the declaration is now free of DeinitializeMemory and InitializeMemory, and the height and width - its now just a constructor, destructor and "mesh", and the memory allocation is done in the constructor and destructor.

This is the new "mesh":

TerrainCell ** mesh;


Mesh::Mesh()
{
mesh = new TerrainCell * [MESH_HEIGHT];
for ( unsigned int i = 0; i < MESH_HEIGHT; ++i)
mesh = new TerrainCell;
}

Mesh::~Mesh()
{
for ( unsigned int i = 0; i < MESH_HEIGHT; ++i )
{
delete [] mesh;
}
delete mesh;
}



Thanks again.

Share this post


Link to post
Share on other sites
Quote:
actually, I changed it, and it is now giving me "invalid pointer" errors at runtime on my linux box, where I am testing the memory leaks.


Then that means you are doing Bad Things™ with the memory, writing where you shouldn't be writing, clobbering data structures that you shouldn't be clobbering, overwriting pointers with non-address data.

That's might be what caused the problems before: you might be doing Bad Things™ with the outer vector and be corrupting the inner vectors, which then don't end up releasing anything.

Or it could be something else entirely. This is mere speculation. Your mileage may vary. Pay no attention to the programmer behind the keyboard. Based on a 1125 calorie diet.

Share this post


Link to post
Share on other sites
actually, all I am doing ( and the operation gives successful results,although they may be the Bad Things you speak of ) is, in the test main function that gets called like 100 times during leak testing is creating a new TerrainCell on the stack, setting its parameters, and using two functions that I hadn't mentioned to put it into the mesh and retrieve it. Here they are:


bool Mesh::SetCell( unsigned int x, unsigned int y, TerrainCell new_cell)
{
if ( x < MESH_WIDTH && y < MESH_HEIGHT )
{
mesh[x][y] = new_cell;
return true;
}
return false;

}

bool Mesh::GetCell( unsigned int x, unsigned int y, TerrainCell & get_cell)const
{
if ( x < MESH_WIDTH && y < MESH_HEIGHT )
{
get_cell = mesh[x][y];
return true;
}
return false;
}



And they are called here:

int mymain(int argc, char *argv[])
{
Mesh terrain_mesh;
TerrainCell new_cell;

for ( unsigned int i = 0; i < 3; ++i )
{
new_cell.position.x = rand()%10;
new_cell.position.y = rand()%10;
new_cell.position.z = rand()%10;
}
if ( terrain_mesh.SetCell(9, 4, new_cell) && terrain_mesh.GetCell(9, 4, new_cell) )
{
std::cout << "Finished successfuly\n";
}
else
std::cout << "Failed!\n";

return 1;
}

Share this post


Link to post
Share on other sites
Your access functions do not use the same width/height order as your constructor and destructor.

Constructor: M[HEIGHT][WIDTH]
Access: M[WIDTH][HEIGHT]

That's a Bad Thing™

Share this post


Link to post
Share on other sites
shouldn't matter, although thanks - since the width and height are the same. I think the problem is that I am making it get a pointer to an object on the stack,so it gets deleted twice - once at the end of the function automatically, and once through delete in the destructor of the Mesh. Let me test it.

[EDIT]Nope. :( Removing the body of mymain didn't affect the program, either. I think its either the constructor or destructor of Mesh.


int mymain(int argc, char *argv[])
{
Mesh terrain_mesh;
std::cout << "Done\n";
return EXIT_SUCCESS;
}



Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement
×

Important Information

By using GameDev.net, you agree to our community Guidelines, Terms of Use, and Privacy Policy.

We are the game development community.

Whether you are an indie, hobbyist, AAA developer, or just trying to learn, GameDev.net is the place for you to learn, share, and connect with the games industry. Learn more About Us or sign up!

Sign me up!