• Status: Solved
  • Priority: Medium
  • Security: Public
  • Views: 321
  • Last Modified:

Memory leakage problem

Hi,
We are having a C++ converter which converts messages into a specific format based on the input.
The problem is whenever this program is called with large number of records, it is taking up huge memory ( > 500 MB) and later on crashes.
There is no issue if the input data is less.

I've identified some point in the code where it might go wrong, can anyone pls help in this issue ---

CONVERTER_OBJS* pGConvObjs = new CONVERTER_OBJS();
    if(!pGConvObjs)
        {
                Msg(szCallerID, "Create Context structure failed");
                return FAILURE;
        }
        memset(pGConvObjs, 0, sizeof(CONVERTER_OBJS));

iRet = SetTemplate(pGConvObjs);

In SetTemplate function, the error is being thrown when trying to load dynamic libraries.

In header file, we have defined --
struct CONVERTER_OBJS
{
    char                                szUniqueName[C_MAX_PATHNAME];
        CDataRep*                       pGDataRep;
        CEnvironment*           pGEnv;
        CSysLog*                        pGErr;
        CProfile*                       pGProfile;
        CDBAccess*                      pGDBAccess;
        CTemplateBuilder*       pGInTmpt;
        CTemplateBuilder*       pGOutTmpt;
    CGVarFuncTable*             pGVarFunc;
        CInBound*                       pGInBound;
        COutBound*                      pGOutBound;
        char*                           pBindVar;
        int                                     iNumBindVar;
    HINSTANCE                   outPgxHandle;
    HINSTANCE                   inPgxHandle;
};

Is there any problem, in initializing the memory with the above memset(),
or should it be done like this ---
memset(pGConvObjs, '\0', sizeof(pGConvObjs));

Pls advise.
0
prasen120998
Asked:
prasen120998
  • 3
  • 3
  • 2
  • +4
3 Solutions
 
novitiateCommented:
there are many pointers in the structure CONVERTER_OBJS, did you verify that they are getting destoryed?

You can consider adding a destructor to your structure.

_novi_
0
 
NadavrubCommented:
[*] DLL Loading problem:
A. Usage of large amount of memory e.g. 500Mb may cause the heap to be fragmented.
B. Loading a DLL into memory require certain amount of continues memory.

Taking in mind what was just said, large amont of fragmented memory may cause the heap to have no free pages to load the DLL, this will cause LoadLibrary to fail.

Resolution: memory allocations should occur as less as possible ( e.g. call new as less as possible ), rather allocate a stack of memory blocks ( or structures ) such as CONVERTER_OBJS on startup and use them when needed, keep them allocated until the application terminates, if not enough blocks are available allocate new blocks and add them to the stack, try to make as less as possible redundant new-delete calls.
Refer to the lookasidelist mechanism for better explanation of the mechanism I have described ( search the DDK or the net for 'ExInitializeNPagedLookasideList' ).
- using the approach I have described may solve the LoadLibrary problem but will not neccesarly solve the memory consumption problem.

[*] Concerning the memset usage:
The way you set the memory is OK.
using 'memset(pGConvObjs, '\0', sizeof(pGConvObjs))' will only set the first four bytes of 'pGConvObjs' to Zero as 'pGConvObjs' is a pointer and the size of any pointer is four.

[*] Concerning the leak:
1. Make sure that when deleting an object of type CONVERTER_OBJS you delete ( if needed ) all the referenced objects ( e.g. pGDataRep, pGEnv, ... )
2. Make sure all of the tasks are not executed simultaneously, this will lead to large memory consumption. if this is the case, try to serialize the tasks execution e.g. execute no more then X tasks together, this could be easily implemented by a thread-pool mechanism or so...

- Also, you can use tools such as Numegas BoundsChecker to identify the leak.

Hope this helps,
    Nadav.
0
 
prasen120998Author Commented:
Can you pls elaborate what you meant by adding some destructors to the structure.
0
Concerto Cloud for Software Providers & ISVs

Can Concerto Cloud Services help you focus on evolving your application offerings, while delivering the best cloud experience to your customers? From DevOps to revenue models and customer support, the answer is yes!

Learn how Concerto can help you.

 
AxterCommented:
I recommend you use the name of your variable when you use memset to zero out a structure.  It's safer, because if you ever change the structure you want to use with that variable, the code will still work.
In your case, since the variable is a pointer, you need to dereference it.

Example:
memset(pGConvObjs, 0, sizeof(*pGConvObjs));

In Win32 programming you can also use ZeroMemory.

ZeroMemory(pGConvObjs, sizeof(*pGConvObjs));

It's more explicit to the reader as what you're trying to do.
0
 
prasen120998Author Commented:
Regarding destroying the pointers in the structure, all the pointers has been deleted except for pGEnv, as it has been used globally
0
 
AxterCommented:
>>Regarding destroying the pointers in the structure, all the pointers has been deleted except for pGEnv, as it has been used globally

You can try using _CrtDumpMemoryLeaks function to try and determine where your leaks are at.
0
 
NadavrubCommented:
I would recommand you to control the life cycle of your objects using reference count, with this concept each object manage it's own life cycle, hence, as long as the object is used it will not be removed from memory AND it will be removed from memory at the minute it is not bing used any more.

To enable reference counting to your objects do the following:
[1] Add a local reference count variable to each of your objects ( e.g. CDataRep, CEnvironment, ... ).
[2] Add each of the objects the following methods:
class MyObj
{
    LONG m_lRefCnt;
    ...
    ...
public:
    inline LONG AddRef() { return InterlockedIncrement(&m_lRefCnt); }
    LONG Release()
    {
        LONG lRef = InterlockedDecrement(&m_lRefCnt);
        if(0 == lRef)
             delete this;
        return lRef;
     }
};
[3] Upon initialization ( e.g. in the constructor or creator function ) set the ref count to zero.
[4] Each time the object is reffered to ( or used ) by another object call AddRef and each time the object stops being used call Release.

Using this logic will gurantee the object will NOT stay in memory whe no one use it AND will garuntee that the object will stay in memory as long as it is being used.

In your case I would recommand using this concept with all of the types referred to by CONVERTER_OBJS with pointers, at the destructor of CONVERTER_OBJS call Release on all of these objects.
0
 
AxterCommented:
You can get that type of logic, without putting it into your class.
Just use a reference counting smart pointer wrapper class.
See following example:
http://code.axter.com/auto_ref_ptr.h

Use it similar to the STL std::auto_ptr smart pointer.

In general, Smart pointers delete the object automatically when smart pointer goes out of scope, so you don't have memory leaks.

However reference counting smart pointers delete the object when the reference counter hits zero.

In either case, you avoid memory leaks, but the reference counting smart pointer is more efficient in general.
0
 
cupCommented:
It is not safe to memset something that has been newed.  If you add a class to that structure, you'll be completely screwed when you do a memset.  If you want to memset it, forget about C++, just use malloc.  If you want to use new, ininitialize the structure (which is actually class { public: ...}) in the constructor.
0
 
prasen120998Author Commented:
I've not used _CrtDumpMemoryLeaks function before, so not sure how to use this function, could you pls elaborate on this.
how it will work in identifying the mem leaks in the code.
0
 
havman56Commented:
use resource allocation is initialization pattern

all ur problemget solved

u can find lot of info on net.

0
 
waysideCommented:
No comment has been added to this question in more than 21 days, so it is now classified as abandoned..
I will leave the following recommendation for this question in the Cleanup topic area:

Split between Nadavrub, Axter, and cup

Any objections should be posted here in the next 4 days. After that time, the question will be closed.

wayside
EE Cleanup Volunteer
0

Featured Post

Upgrade your Question Security!

Add Premium security features to your question to ensure its privacy or anonymity. Learn more about your ability to control Question Security today.

  • 3
  • 3
  • 2
  • +4
Tackle projects and never again get stuck behind a technical roadblock.
Join Now