Re: Memory Leak Questions
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)?
Re: Memory Leak Questions
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.
Re: Memory Leak Questions
Quote:
Originally Posted by ahoodin
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.
Re: Memory Leak Questions
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
Quote:
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".
Re: Memory Leak Questions
So what should i do ? the code that ahoodin gave me, i can not make it compile.
Re: Memory Leak Questions
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.
Code:
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.
Re: Memory Leak Questions
Quote:
Originally Posted by
ahoodin
\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.
Re: Memory Leak Questions
Quote:
Originally Posted by
TheCPUWizard
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.
Quote:
Originally Posted by
TheCPUWizard
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.
Re: Memory Leak Questions
Quote:
Originally Posted by
TheCPUWizard
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.
Quote:
Originally Posted by dada27
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.
Re: Memory Leak Questions
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.
Re: Memory Leak Questions
Quote:
Originally Posted by
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.
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.
Re: Memory Leak Questions
Quote:
Originally Posted by
TheCPUWizard
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
Re: Memory Leak Questions
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.
Quote:
Originally Posted by
Paul McKenzie
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.
Re: Memory Leak Questions
Quote:
Originally Posted by
exterminator
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...
Quote:
To be a proficient programmer...
Again, the same...The issue is what to learn FIRST. NOT what to learn to be "proficient"
Quote:
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...
Quote:
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"...
Quote:
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!
Re: Memory Leak Questions
Quote:
Originally Posted by
TheCPUWizard
that is not what this topic is about. We are talking about a student at a very early point in the learning processs...
I don't think it's fair to assume that the OP is still in the beginning stages of his education, so the topic should have little to do with that. If this is a project for an introductory course, I'm inclined to say that the professor is totally out of his mind... in an admirable way.
I agree with your concerns regarding C++ education, but I've seen the argument in a lot of threads and this thread is no better a soap-box than the last. I don't think the chairperson of any offending institution is following our conversation.
Quote:
Originally Posted by
dada27
Do i need to delete all the SettingsManager::CommandParameter *Pointers that i have on the DBInterface Header, on the Destruction of the DBInterface ???
It depends. The basic principle of memory management is that if you allocate memory, you must know exactly how it gets deallocated - including the case where an exception alters your program execution. If you expect someone else to delete it (a very bad design in most cases), you must clearly say so in the documentation. Conversely, if you are handed a piece of memory by third party code (in which case you are a victim of bad design), you should always consult their documentation.
So, the question I have to ask before I can reasonably give an answer is what allocates the memory in question and what is the expectation at that point as to how it gets deallocated?
There's a postscript to all of this, which is basically what most of the discussion in this thread has been about: You should know how to manage memory, but in practice manual memory management should be avoided at all costs in code that is not specifically designed for the purpose of memory management. A database interface is not a memory management scheme, so do not make it so. Use STL containers and smart pointers for that purpose if at all possible (but as exterminator pointed out, this is not always an option).
As a final note, unless you switch from raw pointers to containers and smart pointers (which you should), you must either implement a public/protected copy constructor and assignment operator, or declare them private and leave them unimplemented.
Re: Memory Leak Questions
As you can see I am a true beginner in C++. I downloaded this DLL which is IAS extension. What this code does is:
I receive an authentication request from my Cisco. Into the RadiusExtensionProccess Function. i get a pointer with all the info of the call. There are a few fucntions that goes trhu the pointer an prepare the CommandParameter and send it to my DB, then the code gets the Return from my Stored Procedure and put it in a pointer and send it back to the Cisco. The DLL that i download have an XML file in where i put the parameters of the SP.
So i needed to call different SP's depending of what i received from my Cisco. so i copied the Example code and renamed the pointers. Everything is working fine, but the SVHOST.exe of the IAS. is going up around 500kb an hour. I thought that maybe the problem was that i am not deleting all the
Code:
SettingsManager::CommandParameter *m_RechargeCardsArray; //Add Recharge Cards
that i rename. But i dont know if i need to delete them. That was my question. Because maybe there is no memory leak, and the SVHOST goes up because i have a lot of a calls going on. i tested with around 25000 calls to the db in one hour.
Can anyone explain me what this code rally does ? g_settings is a class
Code:
// Create the thread-local DB Interface object if not done
DBInterface *dbI = (DBInterface*) TlsGetValue(_tlsDBInterface);
if (!dbI) {
dbI = new DBInterface(*g_settings);
TlsSetValue(_tlsDBInterface,dbI);
}
An then do you see in the code that i first sent. if i need to delete something else or if like that is ok ?
Thank you all
Re: Memory Leak Questions
I think you have a more complicated problem or may be no problem at all. But I can't say for sure as I don't have knowledge of the relevant code and may be you haven't posted the relevant code too.
Quote:
Originally Posted by
dada27
As you can see I am a true beginner in C++. I downloaded this DLL which is IAS extension.
So, this is a downloaded DLL and you have said that you think this DLL has a leak. Well, from the amount of code you have posted till now, I don't see any allocations being done except for DBInterface that contains those pointers. But then inside DBInterface or outside I don't see much happening with those pointers. They are just being initialized to NULL in the constructor and no allocations are being done. The delete[] calls in the destructor hence seem to be deleteing nothing. Although delete call on NULL pointers is okay, but you don't have actually to free anything as the code shown doesn't allocate those arrays. Is there any code that does the allocations? Is it the DLL code that does allocations for those pointers or is it your application code that does it? If the DLL does it, then when it is done with the class' object (that time may vary on different conditions I refer to in my post further), does it free it?
Quote:
Originally Posted by
dada27
What this code does is:
I receive an authentication request from my Cisco. Into the RadiusExtensionProccess Function. i get a pointer with all the info of the call. There are a few fucntions that goes trhu the pointer an prepare the CommandParameter and send it to my DB, then the code gets the Return from my Stored Procedure and put it in a pointer and send it back to the Cisco. The DLL that i download have an XML file in where i put the parameters of the SP.
Those functions are important. They seem to be doing the allocations for all CommandParameter pointer members in DBInterface. How frequently are they allocated? On every request? Are they ever cleaned up apart from the destructor for each request?
Quote:
Originally Posted by
=dada27
So i needed to call different SP's depending of what i received from my Cisco. so i copied the Example code and renamed the pointers. Everything is working fine, but the SVHOST.exe of the IAS. is going up around 500kb an hour. I thought that maybe the problem was that i am not deleting all the
Code:
SettingsManager::CommandParameter *m_RechargeCardsArray; //Add Recharge Cards
that i rename. But i dont know if i need to delete them. That was my question. Because maybe there is no memory leak, and the SVHOST goes up because i have a lot of a calls going on. i tested with around 25000 calls to the db in one hour.
May be, may be not. If the DLL is allocating for those CommandParameters upon every request and doesn't free them, then it's buggy. If it does free them (apart from in the destructor), then may you code has the leak outside of the DLL.
Quote:
Originally Posted by
dada27
Can anyone explain me what this code rally does ? g_settings is a class
Code:
// Create the thread-local DB Interface object if not done
DBInterface *dbI = (DBInterface*) TlsGetValue(_tlsDBInterface);
if (!dbI) {
dbI = new DBInterface(*g_settings);
TlsSetValue(_tlsDBInterface,dbI);
}
Well, I ignored this part and probably everyone else did. You have threads involved which makes it more difficult. What this is doing is:
1. You have a TLS index stored in a global or static _tlsDBInterface variable.
2. Each of the thread that does through the above code does allocate a DBInterface object with g_settings as an argument and it does once when the above code is called.
3. Since the above is in a thread say A, then before thread A finishes, it has got to delete the DBInterface object (and any contained dynamically allocated objects), otherwise, repeated new threads being created would be creating that object and not freeing it and resulting in a leak.
4. If the above is being called from the same persistent thread (without the thread being destroyed), then the above object is just allocated once per-thread and after the first time, it used the TLS Index to recapture the object in its own thread local storage and use the object. At most if no more threads are destroyed and create and execute the above call, it would result in just one object being lost/leaked and you shouldn't see memory usage going up by much in due course of time your application keeps running.
So, now the questions are:
1. How frequently are new threads being created?
2. Is the thread executing above code, does ever free DBInterface object created with new before exiting? If not, you have a leak.
3. I am sure CommandParameters are being created everytime a DBInterface object is being used and prepared for save to DB. But does that happen on each request. Are they ever being freed? How frequently? Do the DBInterface object allocation or CommandParameter objects out-living the thread that creates them? Then you have a leak.
Quote:
Originally Posted by
dada27
An then do you see in the code that i first sent. if i need to delete something else or if like that is ok ?
Since, you say you are using some other DLL. I suppose it is that DLL writer's responsibility to manage resources it creates. His code might be having leaks or is imperfect. In that case, if the author has support, contact them, if not then you will have to fix it on your own. Or use another vendor/another similar DLL that doesn't have such problems (remember I am not saying that this DLL does have a problem because you haven't shown us enough code allocating the resources and their free-ing and thread lifetimes when they are created etc). And yes, to prevent leaks created by dynamic allocations done by 'new' and not being deleted by 'delete' (delete for new and delete[] for new[]), you have to add delete calls to fix it. You will have to carefully understand the object lifetimes, from their construction to till when are they going to be used. They should be freed at the end of their lifetime/usage but not before that.
Overall, this looks non-trivial. You will have to analyse the code much more to get to the actual issue. Threads have made the problem more complex because you the object lifetime may vary depending upon how the threads are being managed and you should have a clear understanding of how each execution path will progress to clearly track the object lifetimes. You may need deallocations at thread exit or if threads are not being created per-request and same one is being used then atleast deallocate per request after use and before overwriting them with newly allocated CommandParameter arrays.
May be there are some tools on windows usable with VS/VC++ (if that's what you are using) that can detect leaks. I haven't see ahoodin's link but if that is helpful - try it out to find if there are leaks and then figure out how to fix them up based on some random hints I gave above.
Hope it helps.
Re: Memory Leak Questions
exterminator Thank you ver much for your help.
I think that in here, Freeing DBInterface.
Code:
HANDLE hEventSource = NULL;
DWORD _tlsDBInterface = 0L;
SettingsManager *g_settings = NULL;
BOOL APIENTRY DllMain( HANDLE hModule,
DWORD ul_reason_for_call,
LPVOID lpReserved
)
{
#ifdef DEBUG
int tmpFlag;
char *leakBuf;
#endif
switch (ul_reason_for_call)
{
case DLL_PROCESS_ATTACH:
#ifdef DEBUG
tmpFlag = _CrtSetDbgFlag( _CRTDBG_REPORT_FLAG );
tmpFlag |= _CRTDBG_CHECK_ALWAYS_DF;
tmpFlag |= _CRTDBG_LEAK_CHECK_DF;
_CrtSetDbgFlag( tmpFlag );
leakBuf = (char*) malloc(128);
strcpy(leakBuf,"Leak check working. Not a leak.");
#endif
hEventSource = RegisterEventSource(NULL,_T("RadiusDB"));
if (hEventSource == NULL)
return FALSE;
_tlsDBInterface = TlsAlloc();
if (_tlsDBInterface == -1)
return FALSE;
break;
case DLL_THREAD_ATTACH:
break;
case DLL_PROCESS_DETACH:
if (hEventSource) DeregisterEventSource(hEventSource);
{
DBInterface *dbI = (DBInterface*) TlsGetValue(_tlsDBInterface);
if (dbI)
delete dbI;
}
if (_tlsDBInterface != -1)
TlsFree(_tlsDBInterface);
if (g_settings) delete g_settings;
break;
case DLL_THREAD_DETACH:
{
DBInterface *dbI = (DBInterface*) TlsGetValue(_tlsDBInterface);
if (dbI)
delete dbI;
}
break;
}
return TRUE;
}
If i am freeing DBInterface at the DLL_THREAD_DETACH, That will delete all of the obejcts that i created inside that class. ?
Re: Memory Leak Questions
The above looks good except that I don't see the DBInterface allocation in DLL_THREAD_ATTACH. (the snippet you posted earlier and asked about what it was doing).
Quote:
Originally Posted by dada27
If i am freeing DBInterface at the DLL_THREAD_DETACH, That will delete all of the obejcts that i created inside that class?
Now, that depends on how DBInterface is implemented. It will or it may not. You haven't posted how DBInterface pointers are allocated and whereall they are freed except that for in the destructor. It might be possible that you are doing allocation for them multiple times during the same DBInterface object lifetime (which is from thread entry in the DLL to its exit from there) and don't release those allocations (or may be just one of them during destruction in DLL_THREAD_DETACH) and hence resulting in a leak. If not (not multiple allocations for the same DBInterface object and if you delete all allocated memory for the pointer members in the destructor), then there isn't a leak at this place. Again, assuming your threads are exiting cleanly.
Re: Memory Leak Questions
May be i found where the leak is, but i am not sure.
Code:
ADODB::_ConnectionPtr connPool;
DWORD WINAPI RadiusExtensionInit ( VOID ){
connPool.CreateInstance( __uuidof( ADODB::Connection ) );
connPool->CursorLocation = ADODB::adUseClient;
connPool->CommandTimeout=5;
connPool->Open("driver={SQL Native Client};server=;Database=tel_data;UID=;PWD=;Connection timeout=5;Connection Reset=TRUE;",empty,empty,ADODB::adConnectUnspecified);
_variant_t iRecAffected(0L);
connPool->Execute("SELECT 1",&iRecAffected,ADODB::adExecuteNoRecords );
connPool->Close();
}
This function is in the same .cpp that the BOOL APIENTRY DllMain that i post before. The radiusExtensionInit gets executed only when i start the service.
I am creating a global scope ADODB::_ConnectionPtr connPool, that i open it on the radiusExtensionInit and then close it, i do this for the connection pooling to work. But i never delete that ConnPool.
I will have a new connPool Object for each new Thread ?
Do i need to delete that pointer? I cant delete right away because If i dont have any instantiated object the connection pool will be destroyed. "Qoute" From MSDN.
Quote:
However, this means your application needs to keep at least one instance of a Connection object instantiated for each unique user—at all times
Could that be a cause of a leak ?
Thank You
Re: Memory Leak Questions
I think it is pretty obvious where the leak is.
Re: Memory Leak Questions
Ahoodin, Can you tell me please?
Do you think that is on the connPool connection object ? because in debug mode. i did a lot of calls, and the connPool obect was always the same, it had always the same address. So i guess that this object only gets created 1 time.
Thank you
Re: Memory Leak Questions
Can you tell me where do you think the leak is ?
thank you
Re: Memory Leak Questions
Make an attempt to add the stackwalker code to your project, read the readme file and add the 6-8 contiguous lines of code to your project.
If that doesnt work, post your project as it is. No object files (no .obj, .exe, .pch or anything in the debug or release directories). Only .vcproj, .sln,
.cpp, .h files.
