Heap corruption

[Krzysztof Kwiecinski]

Hi, debug error says:

HEAP CORRUPION DETECTED: afer Normal block (...)
CRT detected that the application wrote to memory after end of heap buffer.

this error occurs when i try to delete certain instance. I am pretty sure that im not double deleting things, use proper delete. Heres fragment of code:

// kk: skipping initialization cause everything work fine till now
cSimulation::cSimulation(void)
{
	Cities = new cListT<cCity>();
	Events = new cListT<cEvent>();
	Trucks = new cListT<cTruck>();
	g = new cGenerator();

	actTime = 0.0f;
	nextTime = 1000.0f;
}


cSimulation::~cSimulation(void)
{
	delete Events;
	delete Trucks;
	delete Cities;
	delete g;
}

class cSimulation
{
public:
	cSimulation(void);
	~cSimulation(void);

	int Initialize();
	void Start();

private:
	cListT<cCity>* Cities;
	cListT<cTruck>* Trucks;
	cListT<cEvent>*	Events;
	cGenerator* g;

	double actTime;
	double nextTime;
};

class cCity
{
public:
	cCity(void);
	cCity(int id);

	~cCity(void);

private:
	cListT<cEntryPoint>* EntryPoints;
	cListT<cWorkshop>* Workshops;

	int ID;
	bool routes[4];

	friend class cSimulation;
	friend class cSimulationBuilder;
};

cCity::cCity(int id)
{
	EntryPoints = new cListT<cEntryPoint>();
	Workshops = new cListT<cWorkshop>();
	ID = id;
	routes[0] = false;
	routes[1] = false;
	routes[2] = false;
	routes[3] = false;
}

cCity::~cCity(void)
{
	delete EntryPoints;
	delete Workshops;
}


// kk: cEntryPoint and cWorkshop inherit from cAffiliate, they do not have any //      complex structures 

#pragma once
#include "cListT.h"
#include "cPacket.h"
#include "cEmployee.h"

class cAffiliate
{
public:
	cAffiliate(void);									
	cAffiliate(int id, int emp);
	cAffiliate(int id);

	virtual ~cAffiliate(void);

	int GetID();										// returns entry point identificator

protected:
	cListT<cPacket>* qWait;										// queue for incoming packets
	cListT<cPacket>* qMag;										// queue for packet waiting for truck
	cListT<cEmployee>* empl;

	int ID;
	int employee;
	int freeEmp;
};

cAffiliate::cAffiliate(int id, int emp)
{
	employee = (emp>0)?emp:1;
	freeEmp = employee;
	this->ID = id;
	qMag = new cListT<cPacket>();
	qWait = new cListT<cPacket>();
	empl = new cListT<cEmployee>();
	for(unsigned int i = 0; i < employee; i++){empl->PushFront(new cEmployee());}
}

cAffiliate::~cAffiliate(void)
{
	delete qWait;
	delete qMag;
	delete empl; // kk: error occures after executing this line for delete                     ///Workshops
}

[Disch]

Heap corruption occurs when you dereference a bad pointer. This results in memory being corrupted. However you don't find out about it until later (when memory is cleaned up).

Basically what I'm getting at is that even though you get the error on a delete, that's not where the problem is happening. That's just when it's being discovered. The actual problem is elsewhere.

Off the top of my head, I spot a potential problem:
- You have multiple ctors but you only posted the code for one of them. Are you new'ing those objects in your other ctors as well?

- You don't have any copy constructor or assignment operator. You'll need to write those of you will shallow copy your pointers which is problematic



But really, the big issue is....

- Why are you using new for this in the first place? It just makes your code more complicated and error prone. I would get rid of it altogether and just make qMag, qWait, empl, etc all objects instead of pointers. Then you don't need to worry about new/delete at all.

[Krzysztof Kwiecinski]

missing ctors:

cEntryPoint::cEntryPoint(int id, int emp, double wduTime, double packetTime) : cAffiliate(id, emp)
{
	WDUTime = (wduTime > 0) ? wduTime : 10.0f;
	PacketTime = (packetTime > 0) ? packetTime : 10.0f;
}


cEntryPoint::~cEntryPoint(void)
{
}

I do not have copy ctor or assignment operator cause i do not need one atm.

Im using pointers cause earlier version of this project was using them. I'll try to change them to objects but still i want to know what might be causing this error.

Thanks for reply

[Disch]

I mean.... cAffiliate has 3 ctors:

	cAffiliate(void);	
	cAffiliate(int id, int emp);
	cAffiliate(int id);

But you only showed the code for one of them. Are the other two also using new to allocate those objects? If not, they need to be.

I do not have copy ctor or assignment operator cause i do not need one atm.

It's possible you are accidentally copying objects, which would blow a hole in everything. With these kinds of classes where memory is managed you should always provide a copy constructor and assignment operator. If you really don't need them, then make them private and don't give them a body:

class cAffiliate
{
 //...

private:
  cAffiliate(const cAffiliate&);  // don't give them bodies
  cAffiliate& operator = (const cAffiliate&);
};

This way, if you are accidentally copying the objects somewhere, you'll get a nice compiler error instead of having your program explode.

I'll try to change them to objects but still i want to know what might be causing this error.

It's a bad pointer dereference. It could be anywhere in the program, unfortunately. It's not necessarily in the code you posted. Heap corruption can be really hard to track down.

[Krzysztof Kwiecinski]

I mean.... cAffiliate has 3 ctors:

cAffiliate(void);	
	cAffiliate(int id, int emp);
	cAffiliate(int id);

But you only showed the code for one of them. Are the other two also using new to allocate those objects? If not, they need to be.

They are doing the same thing with qWait, qMag and empl and besides, I'm only using only ctor posted above.

Added copy ctor and assigment operator as you advised me but still i got the same error.

I have narrowed program only to initialization and cleaning up - error still shows up. In debug mode everything was looking fine...
About initialization: functon is reading a text file and creates propper structures - uses only PushFirst and PushBack functions posted below.

template <typename T> cListT<T>::cListT(void)
{
	pHead = new cListElementT<T>();
	pHead->pNext = pHead;
	pHead->pPrev = pHead;
}

template <typename T> cListT<T>::~cListT(void)
{
	cListElementT<T>* pTemp;
	while( pHead->Next() != pHead ){
		pTemp = pHead->Next();
		pHead->pNext = pTemp->Next();
		pTemp->pNext->pPrev = pHead;
		delete pTemp;
	}
	delete pHead;
	pHead = NULL;
}

template <typename T> void cListT<T>::PushFront( T* obj )
{
	if( pHead->Next() == pHead )			// empty list
	{
		pHead->pNext = new cListElementT<T>(obj);
		pHead->pNext->pPrev = pHead;
		pHead->pNext->pNext = pHead;
		pHead->pPrev = pHead->pNext;

		return;
	}

	cListElementT<T>* pTemp = new cListElementT<T>(obj);
	pTemp->pPrev = pHead;
	pTemp->pNext = pHead->pNext;
	pHead->pNext = pTemp;
	pTemp->pNext->pPrev = pTemp;
}

template <typename T> void cListT<T>::PushBack(T* obj)
{
	if( pHead->Prev() == pHead )	// empty list
	{
		pHead->pPrev = new cListElementT<T>(obj);
		pHead->pNext = pHead->pPrev;
		pHead->pNext->pPrev = pHead;
		pHead->pNext->pNext = pHead;

		return;
	}

	cListElementT<T>* pTemp = new cListElementT<T>(obj);
	pTemp->pNext = pHead;
	pTemp->pPrev = pHead->pPrev;
	pHead->pPrev = pTemp;
	pTemp->pPrev->pNext = pTemp;
}