Click here to Skip to main content
15,898,134 members
Articles / Desktop Programming / MFC
Article

Be alert to memory leak when using STL to manage pointer only

Rate me:
Please Sign up or sign in to vote.
1.11/5 (28 votes)
29 Aug 20061 min read 78.3K   9   19
for some novice of STL, like me, who might make some low level errors when trying to release memory

Introduction

  As a beginner a STL, I had lots memory leak problems to deal with, and struggled to figure out what the problems were. There problems happen in the case of trying to use the STL container to manage the pointer only. At the beginning I thought that I did put the ojbects in the container, it turned out that I just put the addresses of my ojbects in.

   Truely STL will take care of everything only we do the right thing I think. To the novice of STL like me,  the problem is I didn't realize that I was just letting the container to manage the poniters only. 

The lession tells me that if we just put the addresses in the container, then it is our responsibility to free the memory which are requested  for the objects before losing the addresses information.


   Usually the bedugger would inform the leak kindly, assuming Visual Studio is used.  It must be that just the addresses were deleted not actually releasing the memory.


<br>For example,we are creating a object class capsulated with stl container vector: 

Example code:

Below is the code doing the "wrong thing"?

typedef std::vector<CObject *> ObjectVector;
class CObjectVector : public CObject
{
 DECLARE_SERIAL(CObjectVector);
// Construction
public:
 CObjectVector();

// Attributes
public:

// Operations
public:

// override
public:
 //{{AFX_VIRTUAL(CObjectVector)
 public:
 virtual void Serialize(CArchive& ar);
 //}}AFX_VIRTUAL
 
// Implementation
public:
 void RemoveObject(int nIndex);
 void RemoveAll();
 void AddObject(CObject object);
 virtual ~CObjectVector();

private:
 ObjectVector m_ObjectVector;
 
 //Object relate
private:

};

CObjectVector::CObjectVector(){
} 
  // before doing this, make sure what are deleted
CObjectVector::~CObjectVector(){
   m_ObjectVector.clear(); // Deletes all elements from the vector, which might be just the addresses information.                           
} 
 // same as above
void CObjectVector::RemoveAll(){   
   m_ObjectVector.clear(); // clear everything in the container
} 
 
void CObjectVector::RemoveObject(int nIndex){
   Vector<CObject*>::iterator  where = m_ObjectVector[nIndex];   
    // Yep, just delete the address
   m_ObjectVector.erase(where);  // Deletes the vector element pointed to by the iterator position
} 
  
 void CObjectVector::AddObject(CObject* object){
    m_ObjectVector.push_back(object);
}

void CObjectVector::Serialize(CArchive& ar){

 if (ar.IsStoring())


 { // storing code

   // store the vector
   /* code below is not correct,just for demostration */
   for(i=0;i<m_ObjectVector.size();i++){
       ar << m_ObjectVector[i];
   }
 
 }
 else
 { // loading code



    RemoveAll();
   /* code below  is not correct,just for demostration */       
    CObject* object;
    for(i=0;i<m_nWordsCount;i++){ 
      ar >> object;
      AddObject(object));
   }

}

Now, below code trys to the right thing:

<PRE>// if the elements of container are the pointers of the objects 
CObjectVector::~CObjectVector(){
   for(int i=0;i<m_ObjectVector.size();i++)
      delete m_ObjectVector[i]; // free memory
   m_ObjectVector.clear(); // Deletes all elements from the vector.

}<P class=MsoNormal style="MARGIN: 0cm 0cm 0pt"</P><P class=MsoNormal style="MARGIN: 0cm 0cm 0pt">// seems alright 
void CObjectVector::RemoveAll(){   
   for(int i=0;i<m_ObjectVector.size();i++)
      delete m_ObjectVector[i];
   m_ObjectVector.clear();
}</P><P class=MsoNormal style="MARGIN: 0cm 0cm 0pt">
// correct? I hope so
void CObjectVector::RemoveObject(int nIndex){
   delete m_ObjectVector[nIndex]; // free one elemet memory
   Vector<CObject*>::iterator  where = m_ObjectVector[nIndex];
   m_ObjectVector.erase(where);  // Deletes the vector element pointed to by the iterator position
}</P>

Final words


That is all, and the other containers(list,map,etc) will have the same situation when you let them manage the pointer/address only.
Just remeber to delete elements (mean delete the actual object) first from the container classes before using operation such as erase(), clear()  because  these operation might just delete the poniters.
Good luck!

Acknowledgement


Thank you, Rolf! Thanks for pointing out some big mistakes

License

This article has no explicit license attached to it but may contain usage terms in the article text or the download files themselves. If in doubt please contact the author via the discussion board below.

A list of licenses authors might use can be found here


Written By
Web Developer
Australia Australia
trying to be good

Comments and Discussions

 
Generalmemory leaks with map of strings Pin
samasimo13-Jun-06 21:15
samasimo13-Jun-06 21:15 
GeneralRe: memory leaks with map of strings Pin
amonlee14-Jun-06 21:54
amonlee14-Jun-06 21:54 
GeneralSTL sucks Pin
STL Hater6-Mar-04 3:10
sussSTL Hater6-Mar-04 3:10 
GeneralRe: STL sucks Pin
Jonathan de Halleux6-Mar-04 4:28
Jonathan de Halleux6-Mar-04 4:28 
GeneralRe: STL sucks Pin
Nemanja Trifunovic6-Mar-04 13:41
Nemanja Trifunovic6-Mar-04 13:41 
GeneralI had this problem before ... Pin
RetarT6-Mar-04 2:00
RetarT6-Mar-04 2:00 
GeneralBe sorry to make a big mistake in my article Pin
amonlee5-Mar-04 21:22
amonlee5-Mar-04 21:22 
GeneralThis is absolutely wrong! Pin
Rolf Schaeuble5-Mar-04 3:22
Rolf Schaeuble5-Mar-04 3:22 
GeneralRe: This is absolutely wrong! Pin
Jörgen Sigvardsson5-Mar-04 5:02
Jörgen Sigvardsson5-Mar-04 5:02 
GeneralRe: This is absolutely wrong! Pin
KevinHall5-Mar-04 5:04
KevinHall5-Mar-04 5:04 
GeneralRe: This is absolutely wrong! Pin
Anonymous5-Mar-04 17:15
Anonymous5-Mar-04 17:15 
GeneralRe: This is absolutely wrong! Pin
amonlee5-Mar-04 21:36
amonlee5-Mar-04 21:36 
GeneralRe: This is absolutely wrong! Pin
hoang.lequoc26-Sep-05 16:49
hoang.lequoc26-Sep-05 16:49 
Generalnothing new ... Pin
Maximilien5-Mar-04 1:24
Maximilien5-Mar-04 1:24 
GeneralRe: nothing new ... Pin
John M. Drescher5-Mar-04 2:57
John M. Drescher5-Mar-04 2:57 
GeneralRe: nothing new ... Pin
Zac Howland18-May-06 6:35
Zac Howland18-May-06 6:35 
John M. Drescher wrote:
I agree. This article did not teach me anything because the author did not explain how the leak occurs and how his code fixes it. A line stating "Remeber, when you delete elements from the container classes using operation such as erase(), clear() you have to delete the elements first." does not even begin to explain the situation. Also I know this is a problem with dynamically allocated objects but I question its use with static objects as the type that is held in the vector is CObject and not CObject*.


The problem is quite simple to explain. The STL containers will dynamically allocate memory for the object type you tell it to contain. So, if you tell it to hold type T:

std::vector<T> TVector;

Any additions and cleanup requiring memory allocation/deallocation will set asside (or remove) space for the T object.

Now, if you change that to a T*, it will now allocate/deallocate memory for the pointer (basically, a long integer). It doesn't know (nor care) what you are putting in it, so long as the types match. Thus, it won't bother deleting your object when the vector goes out of scope.

<br />
// within some bounded scope<br />
{<br />
	std::vector<T*> TPtrVector;<br />
<br />
	// add 3 new T* elements<br />
	TPtrVector.push_back(new T);<br />
	TPtrVector.push_back(new T);<br />
	TPtrVector.push_back(new T);<br />
<br />
	// do some cool stuff with our container<br />
<br />
	// vector cleans itself up as it goes out of scope<br />
}<br />


The above code will cause a memory leak. The 3 elements are created, but never destroyed. The vector will create an array of T* objects (T** if you are keeping count) and delete them when it goes out of scope. However, the T objects that the T*'s are pointing to have been created, but are never destroyed. The correct code should be:

<br />
template<class T> struct Deleter : public unary_function<T, void><br />
{<br />
	void operator() (T x) <br />
	{<br />
		if (x != NULL)<br />
		{<br />
			delete x;<br />
		}<br />
	}<br />
};<br />
<br />
// within some bounded scope<br />
{<br />
	std::vector<T*> TPtrVector;<br />
<br />
	// add 3 new T* elements<br />
	TPtrVector.push_back(new T);<br />
	TPtrVector.push_back(new T);<br />
	TPtrVector.push_back(new T);<br />
<br />
	// do some cool stuff with our container<br />
<br />
	// clean up our objects<br />
	for_each(TPtrVector.begin(), TPtrVector.end(), Deleter<T*>());<br />
<br />
	// vector cleans itself up when it goes out of scope<br />
}<br />


An even better solution (using TR1) would look like this:
<br />
// within some bounded scope<br />
{<br />
	std::vector<std::tr1::shared_ptr<T*> > TPtrVector;<br />
<br />
	// add 3 new T* elements<br />
	TPtrVector.push_back(std::tr1::shared_ptr<T*>(new T));<br />
	TPtrVector.push_back(std::tr1::shared_ptr<T*>(new T));<br />
	TPtrVector.push_back(std::tr1::shared_ptr<T*>(new T));<br />
<br />
	// do some cool stuff with our container<br />
<br />
	// vector cleans itself up when it goes out of scope<br />
	// shared pointers clean up correctly as vector mops up<br />
	// since the destructor is called and the reference count goes to 0<br />
}<br />


As for why to use T* over T ... say you have a polymorphic set of classes:
<br />
class Shape<br />
{<br />
public:<br />
	Shape();<br />
	virtual ~Shape();<br />
<br />
	virtual void Draw() {} // or =0; for a pure virtual<br />
};<br />
<br />
class Circle : public Shape<br />
{<br />
public:<br />
	Circle();<br />
	virtual ~Circle();<br />
<br />
	virtual void Draw() { // code to draw a circle }<br />
};<br />
<br />
class Square : public Shape<br />
{<br />
public:<br />
	Square();<br />
	virtual ~Square();<br />
<br />
	virtual void Draw() { // code to draw a square }<br />
};<br />


Now, if you want to store these in the same array after they are initialized, and use that array to redraw the shapes, you would need to use std::vector<Shape*> MyShapes;. If you just wanted to draw circles, you could have an array declared like std::vector<Circle> MyCirlces;. Basically, choosing between the two depends on what they are being used for.



If you decide to become a software engineer, you are signing up to have a 1/2" piece of silicon tell you exactly how stupid you really are for 8 hours a day, 5 days a week

Zac
GeneralRe: nothing new ... Pin
John M. Drescher18-May-06 6:52
John M. Drescher18-May-06 6:52 
GeneralRe: nothing new ... Pin
Zac Howland18-May-06 7:26
Zac Howland18-May-06 7:26 
GeneralRe: nothing new ... Pin
John M. Drescher18-May-06 12:00
John M. Drescher18-May-06 12:00 

General General    News News    Suggestion Suggestion    Question Question    Bug Bug    Answer Answer    Joke Joke    Praise Praise    Rant Rant    Admin Admin   

Use Ctrl+Left/Right to switch messages, Ctrl+Up/Down to switch threads, Ctrl+Shift+Left/Right to switch pages.