Heap Corruption

[ekisner]

So, I know there are a million threads out there, and it's almost always to do with pointers. I'm at a loss though, I've debugged the code step by step, and everything looks fine to me. Which means I am misunderstanding something.

The program is a dashboard... collects and displays a bunch of information. Since both the amount of information can be different (ie sometimes no information to display) and the information itself can be very different, I elected to use a singly linked list to store the information. Each node contains the string and an X,Y coord. The list itself contains the font and color of the text.

Dummyclass was just written on-the-spot to save you from reading a bunch of extra code... in my actual application, it's an abstract class. The Update function is a pure virtual, which is called via a wrapper in the base class to allow the use of _beginthread(). The following member functions are my multi-threaded implementation.

	void ThreadUpdate(){
		_beginthread(&ParentClass::UpdateWrapper,0,static_cast<void*>(this));
	}

	static void __cdecl UpdateWrapper(void* o){
		static_cast<ParentClass*>(o)->UpdateFunction();
	}

I have comments in the code indicating the line that fails. It is within the TextArray destructor.

When stopped at the breakpoint, the char* has a valid null-terminated value in it.

//////////////////////My node
struct TextArray{
	char* text;
	int X,Y;
	TextArray *next;

	TextArray(char* text,int x, int y){
		this->next=NULL;
		this->text=(char*)calloc(strlen(text),sizeof(char));
		strcpy(this->text,text);
		X=x;
		Y=y;
	}

	~TextArray(){
		if (text!=NULL)
			free(text);      ///////////////////THIS LINE FAILS
		this->next=NULL;
	}
};
///////////////////////The list itself

struct TextArrayList{
	TextArray *head;
	int Nodes;
	HFONT GlobalFont;
	COLORREF GlobalColor;
	TextArrayList(HFONT RefToGlobalFont,COLORREF RefToGlobalColor){
		GlobalFont=RefToGlobalFont;
		GlobalColor=RefToGlobalColor;
		Nodes=0;
		head=NULL;
	}
	TextArrayList(HFONT RefToGlobalFont){
		GlobalFont=RefToGlobalFont;
		GlobalColor=black;
		Nodes=0;
		head=NULL;
	};
	~TextArrayList(){
		TextArray *temp;
		while (head!=NULL){
			temp=head;
			head=head->next;
			delete temp;
		};
		if (head!=NULL)
			delete head;
	}

	int AddToList(TextArray *NewNode){
		if (Nodes==0)
			head=NewNode;
		else{
			TextArray *temp=head;
			while (temp->next!=NULL)
				temp=temp->next;
			temp->next=NewNode;
		}
		Nodes++;
		return Nodes;
	}
	void Clear(){
		while (head!=NULL){
			TextArray *temp;
			temp=head;
			head=head->next;
			delete temp;
		}
		this->head=NULL;
		Nodes=0;
	}
};

/////////////The implementation
class dummyclass {
public:
	dummyclass(){list=(TextArrayList*)calloc(1,sizeof(TextArrayList));};
	~dummyclass(){free(list);};
	void UpdateFunction(){list->Clear();doSomething();};   ////this calls Clear()
private:
TextArrayList *list;
};

I have no idea why I get a heap corruption error when I try to free the char*... it was calloc'd, so I would assume it would need to be free()ed?

[slicedpan]

Can't see what's wrong, why not try using an std::string to hold the text, this should get deallocated correctly when the owning class is destructed.

Also your dummyclass implementation is a bit messed up, perhaps your original code is alright. (for one thing, the constructor for TextArrayList is never called so head is never initialised to a sensible value)

[ekisner]

Ah right... yes the original code initializes everything - I just mashed together a dummy class to demonstrate the process of the update clearing the list before doing something.

Gotta say, really starting to pull my hair out with this one, makes no sense to me why I can't free calloc'd memory.

I've read that threads all share the same heap (which as I understand it is where my list is being allocated to). Do you know if this would be the case?

[slicedpan]

I've read that threads all share the same heap (which as I understand it is where my list is being allocated to). Do you know if this would be the case?

Can't answer that one definitively i'm afraid, but I would imagine it is the case. Did you try swapping the char* for a string? It might help you find the problem. Also

When stopped at the breakpoint, the char* has a valid null-terminated value in it.

This would remain true even if the pointer had already been freed. Freeing the pointer will not change the value of the data stored in the memory at that location. This really seems like its a double-free issue. (Could be wrong though!). Happy bug-hunting.

[ekisner]

Thanks for the interest, I'm going to try replacing the char* with a string (which is going to be a lot of re-coding, I think), and also see if I can't find somewhere else that the string may be getting wiped out before it reaches the clean function.

[ekisner]

Perhaps I may query your insight again.

I wrote a memory debugging function, to help me wrap my head around things, and it has produced some... unexpected results.

struct TextArray{
	char* text;
	int X,Y;
	TextArray *next;

	TextArray(char* text,int x, int y){
		this->next=NULL;
		this->text=(char*)calloc(strlen(text),sizeof(char));
		strcpy(this->text,text);
		X=x;
		Y=y;
		int test=(int)static_cast<void*>(&this->text);
		char buf[250];
		itoa(test,buf,16);
		strcat(buf,"\r\n");
		OutputDebugString(buf);
	}
<snip>

When I step through the debug, each address is the same as the address of the list that contains it. Am I somehow misallocating my memory?

[slicedpan]

Do you mean that the address of the field text is the same as the parent struct? If so then this is to be expected. The layout of the struct in memory means that a pointer to the struct itself points to the first member of that struct. Also using the syntax (&this->text) gives you the memory location of a pointer to a char, not the memory location of the string.

[S G H]

Try setting every variable to 0 after it has been freed. And maybe use new and delete instead of calloc and free?

[ekisner]

Problems were many... after an exceedingly excessive number of _CrtCheckMemory() calls, I was able to zero-in on the problems and identify the causes for each.

Some significant problems included the fact that my destructors weren't being called (since the parent destructor was not marked as virtual), I had a function that was called frequently, which returned a dangling pointer, and there was an instance where I had calloc'd some memory but was trying to delete instead of free.

In any case, the problems are resolved. To anyone looking for a solution to their own heap problems, I found it extremely helpful to put an _CrtCheckMemory call at the start and end of every single function... and then add more to each function where you see the heap corruption show up.