Click here to Skip to main content
15,879,535 members
Please Sign up or sign in to vote.
4.92/5 (12 votes)
See more:
Hello all,
I am a bit of a C++ newbie, so I apologize if this is a simple question.

I have a child class that creates a dynamic array of objects of another class.

The header file uses the following to define the pointer variable:
Passenger * passengerList;


The Constructor for the class is allocating the memory for the array as follows:
passengerList = new Passenger[10];


And finally the destructor has the following code to de-allocate the array:
delete [] passengerList;
passengerList = NULL;


This allocation of memory is producing a memory leak and I can't figure out why. I have stepped through the code, tracked the allocation of the array and watched it pass through the destructor and supposedly get de-allocated. And yet I still have the memory leak. Is there any reason why a delete wouldn't work. Oh, and no there aren't any errors during execution.

Thanks


Christian, I double checked my passenger class and there aren't any pointers used in it. Also, I'm using the following for my leak detection.
This is included:
#ifdef _WIN32
#define _CRTDBG_MAP_ALLOC
#include <stdlib.h>
#include <crtdbg.h>
#endif


Followed with this function call in Main():
#ifdef _WIN32
	_CrtDumpMemoryLeaks();
#endif


I read that this is a proper method, but as I pointed out I am a newbie and am open to better methods.

ASPDOTNETDEV, here is the function that actually uses the array:
const void MagicSchoolBus::addPassenger(const Passenger& p)
{
	int pc = this->GetPassengerCount();
	int mp = this->GetMaxPassengers();
	
	if (!this->GetRunningState())
	{
		if (pc < mp)
		{
			passengerList[pc] = p;
			this->SetPassengerCount(++pc);
			cout << "passenger added" << endl;
		}
		else
		{
			mp = mp + 10;
			Passenger * tempList = new Passenger[mp];
			for (int i = 0; i < pc; i++)
			{
				tempList[i] = passengerList[i];
			}
			delete [] passengerList;
			passengerList = tempList;
			this->SetMaxPassengers(mp);
			passengerList[pc] = p;
			this->SetPassengerCount(++pc);
			cout << "passenger added" << endl;
		}
	}
	else
	{
		cout << "Cannot add passengers while the vehicle is running" << endl;
	}			
}



As one might have already guessed, this is for a programming course I am taking. The project has already been turned in because the due date has come and gone but I refuse to let this program beat me and I am determined to find out what is causing the leak.
Thanks again for the suggestions!

Thanks for the continued help everyone. Based on Aescleal's leak tracking suggestion I am getting the same leaks reported that I showed before so apparently I really do have the leaks.

As requested I have added the Passenger constructors and destructor here:
C#
Passenger::Passenger(void)
{
    passengerWeight = 0;
    strcpy_s(passengerName, "Default Name");
}
Passenger::Passenger(char pName[], int pWeight)
{
    passengerWeight = pWeight;
    strcpy_s(passengerName, pName);
}
Passenger::~Passenger(void)
{
}


Also, here are the complete constructors and destructor for the MagicSchoolBus class:
C#
MagicSchoolBus::MagicSchoolBus(void) : Car(0, 10, "Default MagicSchoolBus")
{
    Vehicle::SetMaxPassengers(10);
    passengerList = new Passenger[10];
    for (int i = 0; i < 10; i++)
    {
        passengerList[i] = Passenger();
    }
}
MagicSchoolBus::MagicSchoolBus(int fCap, int fRate, char name[MAXIMUM_NAME_SIZE]) : Car(fCap, fRate, name)
{
    Vehicle::SetMaxPassengers(10);
    passengerList = new Passenger[10];
    for (int i = 0; i < 10; i++)
    {
        passengerList[i] = Passenger();
    }
}
MagicSchoolBus::~MagicSchoolBus(void)
{
    delete [] passengerList;
    passengerList = NULL;
}



Finally here is how the objects are created in Main():
MagicSchoolBus m1;
MagicSchoolBus m2(50, 5, "Craig's Bus");


Also an interesting point; m1 is the only object that has a memory leak. The m2 object does not, even though the contructors are identical.

Thanks again everyone.


Okay here is another data point and I apologize for not mentioning this before. I also get a memory leak that occurs when the passengerList array is copied during the addPassenger function. Previously, the m2 object wasn't presenting as the source of a memory leak because I think its memory gets destroyed when addPassenger creates the new passengerList and the new passengerList becomes the leak.

I came to this conclusion when I removed the m2.addPassenger calls from Main. When I did this suddenly m2 became the source of a memory leak in addition to m1's leak. I also tried adding enough passengers to m1 to force the creation of a new array and it removed the original m1 leak and created (or transfered depending on your interpretation) the leak to the new instance of the array.

I suspect some kind of scope issue but I'm not sure how to diagnose it.
Thanks


Aspdotnetdev,
As requested here are the SetPassengerCount and SetMaxPassengers functions.

C#
const void Vehicle::SetPassengerCount(const int pc)
{
    passengerCount = pc;
}
const void Vehicle::SetMaxPassengers(const int mp)
{
    maxPassengers = mp;
}


As you can see they are very simple setters so I don't see how they could cause a problem. But then again I am the newb and not the expert; yet.

As for using:
this->SetPassengerCount(++pc);

Is using ++pc generally frowned upon from an industry perspective? Its one of those things I did because I know it works and wasn't necessarily thinking about any perceived poor programming practice.

Per your suggestion I tried setting tempList to NULL but it had no effect.
Thanks


I'm using Visual Studio 2008. I have zipped up the project and put it in the public folder of my Dropbox. It can be downloaded from here:
http://dl.dropbox.com/u/1262192/crgMustang_MemoryLeak_problem.zip[^]

And I just want to apologize ahead of time to anyone who happens to look at this. Its not as clean as I like to have my code but I became obsessed with tracking down this memory leak and worked on it right up until my instructors deadline and wound up submitting it in this unfinished state.
But thanks to anyone who takes the time to look at it. It mean a lot to me to get this stuff right.
Posted
Updated 28-May-10 7:30am
v9

Like Christian said, the problem is either that the Passenger class isn't freeing up some memory or that your profiler is incorrect. This worked fine for me:

C++
int * myInts;
for(int i = 0; i < 1000000; i++)
{
	myInts = new int[100];
	delete[] myInts;
}


Can you provide a similar sample that demonstrates the issue (including your Passenger class)?
 
Share this answer
 
v2
Here are some suggestions:

* Does your Passenger class or struct have any members that are pointers? If so, and these members are allocated space on their own, these pointers will have to be freed/deleted before passengerList.

* Are there any pointers in the base class of Passenger that have been allocated space on their own? They would have to be freed/deleted on their own, as well.

* This might sound obvious, but are there any paths of execution in which the delete [] code is not being executed?
 
Share this answer
 
v2
I had a quick look at your code but I dont have a compiler. If I were you I would comment out most of the code in your main function. Create one magic bus and add a single passenger, does it leak memory? How about if you add another passenger etc?

It may be that one of the other objects you're allocating on the stack is the cause of your problem.

You have a lot of copy by reference going which makes the code harder to follow. If you print a message in the constructor and destructor of Passenger you'll probaly be suprised how often they are called as you step through the code. The Passenger class as it is now doesnt require custom copy constructor or assignment operator though.

I think the problem relates to the fact you're redeclared passengerList in your herichy. Vehicle has one, Car, Airplane, MagicBug, Xwing all have a member called passengerList (and others are repeated). Sometimes it's an array of different sizes, sometimes its a pointer. In MagicBus member functions you usually reference it with 'this->passengerList' but in the constructor and destructor you use 'passengerList'. I'm not sure how the compiler will resolve these, you almost certainly need to have a single array (static size) or pointer (dynamic size) in the base class and no other. Try turning up your warning level and see what the compiler says.

Few other things

When passing a string constant that you're going to copy (like the contsructor of Pasenger) you should use 'const char* blah' not 'char blah[somelength]', all you need to pass is the pointer to the start of the string.

When calling a member function from a base class its not usually necessary to prefix it with the name of the base class. The same goes for using this->. Both of these things can cause unintended consequences with virtual functions. I suspect this is par tof your issue.

I noticed in the Fleets class you dont increment vehicleCount when adding a vehicle.

Its also possible that the reporting from the memory tracker is wrong. Does it give the size of the leaked object? This can be a clue sometimes. Do you have access to linux? valgrind is great for this kind of thing.

Sorry if this reply is a bit of amess but its late and had a few drinks
 
Share this answer
 
You can't call "_CrtDumpMemoryLeaks();" in your main test function, because the destructors for your objects will not have run yet, thereby falsely reporting memory leaks that the destructors have not had a chance to clean up yet.

void main()
{
MyObject x;
_CrtDumpMemoryLeaks();
// x is still live, and will falsely be reported as a memory leak.
}

The correct way to do this is something like:

void test_rig()
{
MyObject x;
}
void main()
{
test_rig(); // x was created, ran, and had a chance to clean itself up.
_CrtDumpMemoryLeaks(); // Now see if there are mem leaks.
}

I think this might even work:
void main()
{
{
MyObject x;
} // x::~MyObject() has a chance to run now.
_CrtDumpMemoryLeaks();
}

I think you're seeing a bunch of memory leaks that just aren't there! So do the correction, and then get back to us, if there still is a problem.
 
Share this answer
 
Comments
crgmustang 31-May-10 16:24pm    
Awesome, we have a winner. I have been fighting for the last week or so trying to find memory leaks that don't really exist. I will definitely keep this in mind for my next assignment and any future programs.

Thanks much Waldemar.sauer and thank you everyone else who offered suggestions. I actually have a bunch of good stuff I can apply to my next programming assignment.

This was my first time using this site and I am very impressed with everyone here. Thanks a bunch.
Hi,
i don't like your following statement:

passengerList[i] = Passenger();

What is it for? The elements of passengerList are already initialized with default constructor of Passenger class. If you omit this, do you still have memory leaks?
Jarek
 
Share this answer
 
Does the destructor for Passenger delete all the objects created inside the class? If it does, it's possible your leak detector just sucks.
 
Share this answer
 
v2
Your code shows that for every time the passenger list has to be reallocated, it will first be allocated, copied, destroyed(original one) and assigned. This all looks good. What your code doesn't show though is how passenger list is initialized along with the variables for passenger count and maximum passengers. Depending upon how that is done, the memory leak could be from that. For example, I'll assume that passenger count and max passengers are intialized to 0 and that passenger list is also NULL. In this case, your list will only be allocated upon the first call to AddPassenger. This will then initialize and create the first instance of a passenger list, but now where will that list every get destroyed? Hope that makes some sense to you. :)

EDIT
Apologies, I focused a little too much on that one method and missed the code up above which I now assume is for the constructor and destructor for your MagicSchoolBus class. Are the two variables set to 0 and 9 in the constructor as well? Also, is the MagicSchoolBus class instantiated on the stack or is it allocated by new as well? Seeing that part of the code might be useful also.

EDIT
For the Passenger class, there looks to be a member called passengerName. What is it's type and what kind of allocation occurs for it?
 
Share this answer
 
v3
Are you checkpointing your heap allocations correctly? if you're not _CrtDumpMemoryLeaks(); will be reporting anything allocated through new or malloc since the program started as leaks.

So try calling _CrtMemCheckpoint(); at the start of main and _CrtMemDumpAllObjectsSince at the end of main. If there any leaks you've caused they'rll show up.

int main()
{
    _CrtMemState starting_state;
    _CrtMemCheckpoint( &starting_state );

    // Much good stuff...

    _CrtMemDumpAllObjectsSince( &starting_state );
}


Cheers,

Ash
 
Share this answer
 
crgmustang wrote:
m1 is the only object that has a memory leak. The m2 object does not, even though the contructors are identical.


Almost identical. Try replacing Car(0, 10, "Default MagicSchoolBus") with Car(50, 5, "Craig's Bus"). See if that changes anything. Also, perhaps the memory isn't leaked by m2 because it is some shared resource that m1 causes to leak first (that "Vehicle::SetMaxPassengers(10)" is a possible culprit). Does m2 leak if you remove m1?
 
Share this answer
 
Can you post the code for SetPassengerCount and SetMaxPassengers? Might be something funky going on in there.

Also, try setting tempList to NULL after you assign it to passengerList. That shouldn't stop a memory leak, but maybe it will prevent the tool you are using from incorrectly reporting a memory leak.

One more thing:
C++
this->SetPassengerCount(++pc);

I'd just increment pc before passing it to SetPassengerCount. That way, you avoid dealing with the differences between "++pc" and "pc++", which may cause problems.
 
Share this answer
 
Well, I'm stumped. What IDE are you using (Visual Studio, Dev-C++, other)? How about you put your entire project online somewhere and so I or somebody else can see everything that's going on?
 
Share this answer
 

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)



CodeProject, 20 Bay Street, 11th Floor Toronto, Ontario, Canada M5J 2N8 +1 (416) 849-8900