Memory Leak Questions

[dada27]

Hi,

I have a dll and i think that has a memory leak.
The programs enter to this function everytime that i need to call the database.

 // Create the thread-local DB Interface object if not done
    DBInterface *dbI = (DBInterface*) TlsGetValue(_tlsDBInterface);
    if (!dbI) {
		#ifdef DEBUG
			REPORT_INFO(EVMSG_TLSINIT,0,NULL);
		#endif
	  dbI = new DBInterface(*g_settings);
	  TlsSetValue(_tlsDBInterface,dbI);
	}      
		ADODB::_RecordsetPtr recSet;
		int returnCode;
		
			recSet = dbI->NA_RD_AccesoRequest(pInAttrs,&returnCode);

This is the constructor and destructos of the class DBInterface.

//////////////////////////////////////////////////////////////////////
// Construction/Destruction
//////////////////////////////////////////////////////////////////////

DBInterface::DBInterface(SettingsManager &smgr) :
m_settings(smgr), m_valid(false)/*, m_LRAttributeArray(NULL)*/,m_NA_RD_LRAttributeArray(NULL),
m_ACAttributeArray(NULL), /*m_GPAttributeArray(NULL),*/m_ACUPAttributeArray(NULL),m_RechPCAttributeArray(NULL),m_RechCCAttributeArray(NULL),
m_passwordOutputIndex(-1L)
{
}

DBInterface::~DBInterface()
{
	//if (m_LRAttributeArray) delete[] m_LRAttributeArray;
	//if (m_GPAttributeArray) delete[] m_GPAttributeArray;
	if (m_ACAttributeArray) delete[] m_ACAttributeArray;
	if (m_ACUPAttributeArray) delete[] m_ACUPAttributeArray;
	if (m_RechPCAttributeArray) delete[] m_RechPCAttributeArray;
	if (m_RechCCAttributeArray) delete[] m_RechCCAttributeArray;
	if (m_NA_RD_LRAttributeArray) delete[] m_NA_RD_LRAttributeArray;
}

this is the header file of the DBInterface.h

// DBInterface.h: interface for the DBInterface class.
//
//////////////////////////////////////////////////////////////////////

#if !defined(AFX_DBINTERFACE_H__4B3AE378_BB57_4521_9AF3_F26A98A681BB__INCLUDED_)
#define AFX_DBINTERFACE_H__4B3AE378_BB57_4521_9AF3_F26A98A681BB__INCLUDED_

#if _MSC_VER > 1000
#pragma once
#endif // _MSC_VER > 1000

class DBInterface  
{
public:
  DBInterface(SettingsManager &sm);
  virtual ~DBInterface();

  // Run a login request
 // ADODB::_RecordsetPtr LoginRequest(CONST RADIUS_ATTRIBUTE *pInAttrs, BSTR *pwd = NULL);
  // Run a acceso request
  ADODB::_RecordsetPtr AccesoRequest(CONST RADIUS_ATTRIBUTE *pInAttrs, int *returnCode = NULL);
   // Run a acceso request IN TEL DATA NA_RD_
  ADODB::_RecordsetPtr NA_RD_AccesoRequest(CONST RADIUS_ATTRIBUTE *pInAttrs, int *returnCode = NULL);

  // Run Recharge PC
  ADODB::_RecordsetPtr RechargePCAuth(CONST RADIUS_ATTRIBUTE *pInAttrs, int *returnCode = NULL);

  // Run Agrega cuentas pc
  ADODB::_RecordsetPtr AccountsPCAuth(CONST RADIUS_ATTRIBUTE *pInAttrs, int *returnCode = NULL);
 
  // Run Check Balance pc
  ADODB::_RecordsetPtr CheckBalancePC(CONST RADIUS_ATTRIBUTE *pInAttrs, int *returnCode = NULL);

   // Run Recharge Cards
  ADODB::_RecordsetPtr RechargeCards(CONST RADIUS_ATTRIBUTE *pInAttrs, int *returnCode = NULL);

  // Run Add Numbers pc
  ADODB::_RecordsetPtr AddNumbersPC(CONST RADIUS_ATTRIBUTE *pInAttrs, int *returnCode = NULL);

  // Run Recharge CC
  ADODB::_RecordsetPtr RechargeCCAuth(CONST RADIUS_ATTRIBUTE *pInAttrs, int *returnCode = NULL);

  // Run a password fetch request
 // BSTR GetPassword(CONST RADIUS_ATTRIBUTE *pInAttrs);
  // Handle an accounting stop packet
  void AccountingRequest(CONST RADIUS_ATTRIBUTE *pInAttrs);

  // Handle an accounting Update packet
  void AccountingUpdate(CONST RADIUS_ATTRIBUTE *pInAttrs);

  BOOL isValid();
protected:
  BOOL m_valid;
  
  ADODB::_CommandPtr m_CheckBalancePCCommand;
  _bstr_t            m_CheckBalancePCConn;
  ADODB::_CommandPtr m_accesoRequestCommand;
  _bstr_t            m_accesoRequestConn;
  ADODB::_CommandPtr m_NA_RD_accesoRequestCommand;
  _bstr_t            m_NA_RD_accesoRequestConn;
  ADODB::_CommandPtr m_RechargePCAuthCommand;
  _bstr_t            m_RechargePCAuthConn;
  ADODB::_CommandPtr m_AddNumbersPCAuthCommand;
  _bstr_t            m_AddNumbersPCAuthConn;
  ADODB::_CommandPtr m_AccountsPCAuthCommand;
  _bstr_t            m_AccountsPCAuthConn;
   ADODB::_CommandPtr m_RechargeCCAuthCommand;
  _bstr_t            m_RechargeCCAuthConn;
 /* ADODB::_CommandPtr m_loginRequestCommand;
  _bstr_t            m_loginRequestConn;*/
  ADODB::_CommandPtr m_accountingCommand;
  _bstr_t            m_accountingConn;
  ADODB::_CommandPtr m_AccountingUpdateCommand;
  _bstr_t            m_AccountingUpdateConn;
  //ADODB::_CommandPtr m_getPasswordCommand;
  //_bstr_t            m_getPasswordConn;
  ADODB::_CommandPtr m_RechargeCardsCommand;
  _bstr_t            m_RechargeCardsConn;

  _variant_t         m_passwordOutputIndex;
  _variant_t         m_loginPasswordOutputIndex;
  
 // SettingsManager::CommandParameter   *m_GPAttributeArray;
 // SettingsManager::CommandParameter   *m_LRAttributeArray;
  SettingsManager::CommandParameter   *m_NA_RD_LRAttributeArray; //ARRAY PARA EL ACCESSO REQUEST DE TEL_DATA
  SettingsManager::CommandParameter   *m_ACAttributeArray;
  SettingsManager::CommandParameter   *m_ACUPAttributeArray;
  SettingsManager::CommandParameter   *m_RechPCAttributeArray; //RECHARGE PC
  SettingsManager::CommandParameter   *m_RechCCAttributeArray; //CREDIT CARDS
  SettingsManager::CommandParameter   *m_AccountsPCAttributeArray; //Accounts PC
  SettingsManager::CommandParameter   *m_CheckBalancePCAttributeArray; //Balance of  PC
  SettingsManager::CommandParameter   *m_AddNumbersPCAttributeArray; //Add numbers from  PC
  SettingsManager::CommandParameter   *m_RechargeCardsArray; //Add Recharge Cards
   
  SettingsManager &m_settings;

  // Execute an ADO command and return the recordset
  ADODB::_RecordsetPtr CallCommand(ADODB::_CommandPtr cp, 
																	 _bstr_t &connStr,
																	 SettingsManager::CommandParameter *attrArray,
																	 CONST RADIUS_ATTRIBUTE *pAttrs);
};

#endif //

Do i need to delete all the SettingsManager::CommandParameter *Pointers that i have on the DBInterface Header, on the Destruction of the DBInterface ???

Could that be the memory leak ?

Thank you

[Lindley]

Looks to me like overuse of pointers. Why not try replacing all dynamic arrays with std::vectors, and other raw pointers by smart pointers (typically boost::shared_ptr or std::tr1::shared_ptr)?

[ahoodin]

Naaah just fix your leaks.

http://www.codeproject.com/KB/applic...eakfinder.aspx

This app will return the exact line with your leak or exception in
a nice XML format and includes a reader that will make it even easier
to see where your leak is.

[laserlight]

Naaah just fix your leaks.

Heh, but Stroustrup points out that one way to fix your memory leaks is by writing code that doesn't have any, via the systematic use of containers and smart pointers.

[TheCPUWizard]

A "correct" program works because of the combination of two factors.

1) Works by Design
2) Works by Usage

Remember the Scott Meyers quote from the early 1990's

Design your classes so they are easy to use properly, and difficult to use improperly

(emphasis added)

THis highlights the benefits of "Works by Design".

In the specific current context, the usage of raw pointers and/or arrays is an implementation that "Works by Usage", while usage of std::vector (et. al.), Smart Pointers (any variety) are all examples of "Works by Design".

[dada27]

So what should i do ? the code that ahoodin gave me, i can not make it compile.

[ahoodin]

Follow the instructions in the zip file. It has a readme file.

It tells you how to include the code in your project, how to compile your project, even where to add the 6-10 lines of code that make it work.

by writing code that doesn't have any

Yep, thats why he is going to fix his leaks. Just because there is garbage collected code out there doesn't mean that it isn't inefficiently coded. Look at all the inefficently coded .net projects out there. So it is garbage collected at some point. I am sorry but I see memory pooling all around. Sure he should use smart pointers and containers. Your assuming he knows the STL. Maybe I am assuming he doesn't but he also took the time to write this code which may have taken some small effort. Why can't he just learn to fix the code he already wrote? I am not saying he can't learn about smart pointers and all those other methods. He should still learn pointers, and he should still learn to allocate memory efficiently. Also he asked about a leak. Not STL.

[TheCPUWizard]

\Yep, thats why he is going to fix his leaks. Just because there is garbage collected code out there doesn't mean that it isn't inefficiently coded. Look at all the inefficently coded .net projects out there. So it is garbage collected at some point. I am sorry but I see memory pooling all around. Sure he should use smart pointers and containers. Your assuming he knows the STL. Maybe I am assuming he doesn't but he also took the time to write this code which may have taken some small effort. Why can't he just learn to fix the code he already wrote? I am not saying he can't learn about smart pointers and all those other methods. He should still learn pointers, and he should still learn to allocate memory efficiently. Also he asked about a leak. Not STL.

Some very good points...but...

Doesn't one usually learn how to drive a car BEFORE they build one from scratch?

IMPO It sould be much more worthwhile to start with proven components (in this case STL) and once there is a through understanding of how to USE them, expand the knowledge to learn how to PROPERLY implement them using the "native" capabilities.

Also (off-topic) Poor programs can be written in ANY language. .NET is no different. But look at the difference. Poorly written managed application tend to run and produce the desired result. They are "correct" in accordance with the language specification, and execute DEFINED behaviour. Poorly written C++ programs tend to crash, or have "bizarre" results as the direct rsult of undefined behavour. This is another good example of "Works by Design" (.NET) and "Works by Usage" (C++ using "raw" constructs.

[ahoodin]

IMPO It sould be much more worthwhile to start with proven components (in this case STL) and once there is a through understanding of how to USE them, expand the knowledge to learn how to PROPERLY implement them using the "native" capabilities.

Well sure, but I am not certain he has the background at this point in time. If he knew the STL, he might not be asking this question at this particular juncture. Definitely do learn the STL, learn smart pointer.

Also (off-topic) Poor programs can be written in ANY language. .NET is no different. But look at the difference. Poorly written managed application tend to run and produce the desired result.

Well unmanaged code still seems to permeate many architectures. The thing about writing unmanaged code w.o garbage collection is that you learn to clean up after yourself.

[exterminator]

Also (off-topic) Poor programs can be written in ANY language. .NET is no different. But look at the difference. Poorly written managed application tend to run and produce the desired result. They are "correct" in accordance with the language specification, and execute DEFINED behaviour. Poorly written C++ programs tend to crash, or have "bizarre" results as the direct rsult of undefined behavour. This is another good example of "Works by Design" (.NET) and "Works by Usage" (C++ using "raw" constructs.

You took this way off-topic. More than what ahoodin said about. He talked of inefficient programs and you skipped that point without any comment and moved on to start a argument about constructs resulting in undefined behaviour which has nothing to do with efficiency.

While I very strongly agree towards using smart pointers and RAII in general, the developer must know how to fix memory leaks. And what causes memory leak in the first place. The OP doesn't even know if what he coded is a memory leak and he was forwarded to using smart pointers. If I don't know what the purpose of something is and what it fixes and what is being fixed in the first place, why would I even care to look at that thing? Having said that, it depends on the OP to make his/her life easier using well designed constructs for any non-learning work but I would expect him/her to be aware of leaks and how to handle them. You can't just go on switching everything to a smart pointer in say a huge maintenance project unless that is actually what you are being paid for/asked to do.

Do i need to delete all the SettingsManager::CommandParameter *Pointers that i have on the DBInterface Header, on the Destruction of the DBInterface ???

Could that be the memory leak ?

Yes, if you don't clean up the dynamically allocated memory, you do have a memory leak. So, you should ensure that the memory is freed when the use is over. Also, I am not sure but does the DBInterface class own those pointers? If not, then it should not free them. The owner has to take care of this. In case of shared ownership shared_ptr/shared_array types might be helpful to use.

[exterminator]

Also, I don't see a private copy constructor/assignment operator for the class. Is that to be supported or not? If it is not then do as said before and if it is then you would need to provide your own implementation of those members. If the pointers you have used are just to represent arrays then Lindley's suggestion of using growable arrays (like vector) might be a good idea and you will have the relief of not maintaining the 3 (d-tor/copy c-tor/assignment op) everytime you modify/add/remove one member.

[TheCPUWizard]

Also, I don't see a private copy constructor/assignment operator for the class. Is that to be supported or not? If it is not then do as said before and if it is then you would need to provide your own implementation of those members. If the pointers you have used are just to represent arrays then Lindley's suggestion of using growable arrays (like vector) might be a good idea and you will have the relief of not maintaining the 3 (d-tor/copy c-tor/assignment op) everytime you modify/add/remove one member.

However, if Lindley's good advice (in the very first reply on the thread) was taken, then this issue would not even exist, as the implementation would simply "Work by Design". IMPO, yet another example of why the order in which one is exposed to and learns about information is important.

[Paul McKenzie]

However, if Lindley's good advice (in the very first reply on the thread) was taken, then this issue would not even exist, as the implementation would simply "Work by Design". IMPO, yet another example of why the order in which one is exposed to and learns about information is important.

If C++ were introduced to new programmers as books such as "Accelerated C++" or even how Stroustrup himself recommends it should be introduced, then this entire thread may not have existed.

But to what should be learned first -- all I have to say is that I've yet to see a C++ programmer who knows STL (and learned STL first) and at the same time know nothing about pointers and memory leaks. I have seen programmers who know pointers but do not know STL. It's the latter that have a tougher time finding C++ employment positions.

Regards,

Paul McKenzie

[exterminator]

There are maintenance applications where you just can't go on changing pointers to smart pointers because the spread is too complex to cover up when achieving something else.

To be a proficient programmer, one has to understand naked pointers plus smart pointers agreed. But one and not the other is not an option whichever way it is.

It will prevent some beginners from making the same mistakes in using raw pointers but if they don't have good understanding - they can only write good new code and not be able to maintain existing code and even probably not be very quick at working with the 3rd party libs that do so. Hence, I stressed that teaching smart pointers/vectors at the start are good but skipping teaching raw pointer usage is not an option. I haven't gone through Accelerated C++ but I believe if it doesn't at some point teach using raw pointers, it is an incomplete text because not all are lucky enough to just write new code. But that's just my person opinion.

If C++ were introduced to new programmers as books such as "Accelerated C++" or even how Stroustrup himself recommends it should be introduced, then this entire thread may not have existed.

But to what should be learned first -- all I have to say is that I've yet to see a C++ programmer who knows STL (and learned STL first) and at the same time know nothing about pointers and memory leaks. I have seen programmers who know pointers but do not know STL. It's the latter that have a tougher time finding C++ employment positions.

All agreed. Never said I disagree.

But in the context of this thread. If I understood the OP correctly, he wasn't even sure if that was a memory leak. And no one confirmed it to him except that ahoodin's post just made it feel that it was and proposing a way to detect it.

[TheCPUWizard]

There are maintenance applications ....

that is not what this topic is about. We are talking about a student at a very early point in the learning processs...

To be a proficient programmer...

Again, the same...The issue is what to learn FIRST. NOT what to learn to be "proficient"

It will prevent some beginners from making the same mistakes in using raw pointers but if they don't have good understanding...

And that can be accomplished one they have a little more education...

Hence, I stressed that teaching smart pointers/vectors at the start are good but skipping teaching raw pointer usage is not an option.

Now we AGREE. Ad based on the original post, the person is "at the start"...

But in the context of this thread. If I understood the OP correctly, he wasn't even sure if that was a memory leak. And no one confirmed it to him except that ahoodin's post just made it feel that it was...

This goes back to the question of is the OP at the point where he already knows how to accomplish tasks in a manner where memory leaks have not become an issue, and now needs to proceed to the "next level"..

IMPO, If the person can not yet accomplish the task using proven, relatively "safe" techniques, then they are not]yet ready to deal with the more "dangerous" lower level techniques. This has noting to do with the necessity of them learning at the appropriate time.

Again, IMPO, the best situation is when a person states "I have an implementation using STL that works", I am now tring to accomplish the same task using primitives, so I have a better understanding?

An approach of "I am spending my time digging for Oil, and learning how to refine it, so I can figure out how to drive a car, and get my license", simply does not make any sense to me. The person who says, "I have a license, and I am responsible for a lot of cars, so I want to learn and understant Oil exploration and rinfinging to see how gasoline actually gets to my car", is the one who will become a success!