Questions about C++ memory allocation and delete

Question

I'm getting a bad error. When I call delete on an object at the top of an object hierarchy (hoping to the cause the deletion of its child objects), my progam quits and I get this:

*** glibc detected *** /home/mossen/workspace/abbot/Debug/abbot: double free or corruption (out): 0xb7ec2158 ***

followed by what looks like a memory dump of some kind. I've searched for this error and from what I gather it seems to occur when you attempt to delete memory that has already been deleted. Impossible as there's only one place in my code that attempts this delete. Here's the wacky part: it does not occur in debug mode. The code in question:


Terrain::~Terrain()
{
    if (heightmap != NULL) // 'heightmap' is a Heightmap*
    {
        cout << "heightmap& == " << heightmap << endl;
        delete heightmap;
    }
}

I have commented out everything in the heightmap destructor, and still this error. When the error occurs,

heightmap& == 0xb7ec2158

is printed. In debug mode I can step through the code slowly and

heightmap& == 0x00000000

is printed, and there is no error. If I comment out the 'delete heightmap;' line, error never occurs. The destructor above is called from another destructor (separate classes, no virtual destructors or anything like that). The heightmap pointer is new'd in a method like this:


Heightmap* HeightmapLoader::load() // a static method
{
   // ....
   Heightmap* heightmap = new Heightmap();
   // ....other code
   return heightmap;
}

Could it be something to do with returning a pointer that was initialized in the stack space of a static method? Am I doing the delete correctly? Any other tips on what I could check for or do better?

score 2 · Accepted Answer · answered Oct 26 '09 at 04:43

2

What happens if load() is never called? Does your class constructor initialise heightmap, or is it uninitialised when it gets to the destructor?

Also, you say:

... delete memory that has already been deleted. Impossible as there's only one place in my code that attempts this delete.

However, you haven't taken into consideration that your destructor might be called more than once during the execution of your program.

answered Oct 26 '09 at 04:43

Greg Hewgill

951,095
183
1,149
1,285

Yep, that was it. I thought for sure heightmap* was being set (not in the constructor, but in a setter method), alas it was not. Thanks, Greg! – Mossen Oct 26 '09 at 04:59
1

Always initialize all your pointers during construction. A null pointer can be safely deleted (no-op). If your constructor setted the pointer to 0 in the initialization list then the code would be correct from the beginning. – David Rodríguez - dribeas Oct 26 '09 at 06:43
1

And of course there might be copy construction and/or assignment that make two objects pointing to the same memory zone... – Matthieu M. Oct 26 '09 at 08:44

score 2 · Answer 2 · answered Oct 26 '09 at 04:45

2

In debug mode pointers are often set to NULL and memory blocks zeroed out. That is the reason why you are experiencing different behavior in debug/release mode.

I would suggest you use a smart pointer instead of a traditional pointer

auto_ptr<Heightmap> HeightmapLoader::load() // a static method
{
   // ....
   auto_ptr<Heightmap> heightmap( new Heightmap() );
   // ....other code
   return heightmap;
}

that way you don't need to delete it later as it will be done for you automatically

see also boost::shared_ptr

answered Oct 26 '09 at 04:45

AndersK

35,813
6
60
86

Thanks, I did not realize that about debug mode...I'm used to coding in Java...I will look into the auto_ptr! – Mossen Oct 26 '09 at 05:02
+1, also take a look at `scoped_ptr<>` and `unique_ptr<>`, either from boost or std::tr1. They are different solutions to smart pointer memory management. – David Rodríguez - dribeas Oct 26 '09 at 07:03

score 1 · Answer 3 · answered Oct 26 '09 at 04:45

It's quite possible that you're calling that dtor twice; in debug mode the pointer happens to be zeroed on delete, in optimized mode it's left alone. While not a clean resolution, the first workaround that comes to mind is setting heightmap = NULL; right after the delete -- it shouldn't be necessary but surely can't hurt while you're looking for the explanation of why you're destroying some Terrain instance twice!-) [[there's absolutely nothing in the tiny amount of code you're showing that can help us explain the reason for the double-destruction.]]

score 0 · Answer 4 · answered Oct 26 '09 at 04:50

It looks like the classic case of uninitialized pointer. As @Greg said, what if load() is not called from Terrain? I think you are not initializing the HeightMap* pointer inside the Terrain constructor. In debug mode, this pointer may be set to NULL and C++ gurantees that deleting a NULL pointer is a valid operation and hence the code doesn't crash. However, in release mode due to optimizations, the pointer in uninitialized and you try to free some random block of memory and the above crash occurs.

Questions about C++ memory allocation and delete

4 Answers4