Thread: Odd memory leaks

Well I don't want to post 20000 lines of source here. I cannot give more info because I do not have more information.

The problem with destructors is when and if they are called. At first sight this may seem easy to determine but when you add in vectors, MFC objects, COM objects, etc, etc... a destroy function for each object is much simpler to follow.

I like destructors in that you know when the object goes out of scope the destructor will be called. This makes them much better than the ever so popular garbage collector and the gcc_new() of MSVC 2005. However I dislike them because their functionality is inherent in the language design and not ascertainable from the actual source code. That is, you cannot see specifically when the destructor is called. Perhaps a compiler that added a non-editable line of code that showed the call would help. I like COM and COM+'s approach to objects a bit better than C++. I dislike Java and C#'s GC and now MS's CLI GC.

I think this is why MS decided on Release() for COM objects. You can tell exactly when the object is cleaned up by simply looking at the code.

Perhaps it's a bad argument, perhaps not.

Exit() is a function that originated in the DOS only days. The function, which used to execute DOS EXIT through int 21h and by setting AX to 4Fh, has been modified so that it does not rely on DOS, however, it's functionality has not changed much. AtExit() is in the same line of thinking as exit(). There are much better ways to accomplish the same tasks these two perform.

From the Platform SDK - .NET 2005

Terminate the calling process after cleanup (exit) or immediately (_exit).


void exit(
int status
);
void _exit(
int status
);



Parameters
status
Exit status.

Remarks
The exit and _exit functions terminate the calling process. exit calls, in last-in-first-out (LIFO) order, the functions registered by atexit and _onexit, then flushes all file buffers before terminating the process. _exit terminates the process without processing atexit or _onexit or flushing stream buffers. The status value is typically set to 0 to indicate a normal exit and set to some other value to indicate an error.

Although the exit and _exit calls do not return a value, the low-order byte of status is made available to the waiting calling process, if one exists, after the calling process exits. The status value is available to the operating-system batch command ERRORLEVEL and is represented by one of two constants: EXIT_SUCCESS, which represents a value of 0, or EXIT_FAILURE, which represents a value of 1. The behavior of exit, _exit, _cexit, and _c_exit is as follows.

Function Description
exit
Performs complete C library termination procedures, terminates the process, and exits with the supplied status code.

_exit
Performs quick C library termination procedures, terminates the process, and exits with the supplied status code.

_cexit
Performs complete C library termination procedures and returns to the caller, but does not terminate the process.

_c_exit
Performs quick C library termination procedures and returns to the caller, but does not terminate the process.


When you call the exit or _exit functions, the destructors for any temporary or automatic objects that exist at the time of the call are not called. An automatic object is an object that is defined in a function where the object is not declared to be static. A temporary object is an object created by the compiler. To destroy an automatic object before calling exit or _exit, explicitly call the destructor for the object, as follows:

Copy Code
myObject.myClass::~myClass();


You should not call exit from DllMain with DLL_PROCESS_ATTACH. If you want to exit the DLLMain function, return FALSE from DLL_PROCESS_ATTACH.

Requirements
Function Required header Compatibility
exit
<process.h> or <stdlib.h>
ANSI, Windows 95, Windows 98, Windows 98 Second Edition, Windows Millennium Edition, Windows NT 4.0, Windows 2000, Windows XP Home Edition, Windows XP Professional, Windows Server 2003

_exit
<process.h> or <stdlib.h>
Windows 95, Windows 98, Windows 98 Second Edition, Windows Millennium Edition, Windows NT 4.0, Windows 2000, Windows XP Home Edition, Windows XP Professional, Windows Server 2003


For additional compatibility information, see Compatibility in the Introduction.

Example
Copy Code
// crt_exit.c
// This program returns an exit code of 1. The
// error code could be tested in a batch file.

Code:

#include <stdlib.h>

int main( void )
{
   exit( 1 );
}

.NET Framework Equivalent
System::Diagnostics::Process::Kill

See Also
Reference
Process and Environment Control
abort
atexit
_cexit, _c_exit
_exec, _wexec Functions
_onexit, _onexit_m
_spawn, _wspawn Functions
system, _wsystem

I haven't used exit() in a very very long time as there are now much better ways to terminate an application and/or exit early from an application.

These memory leaks are being caused by something else internal to my code. The debugger cannot determine the source file because it's not a matter of a simple new w/o corresponding delete.

For instance if you exit an MFC app and attempt to destroy a DC that has a CBitmap selected into it, you will leak the CBitmap if you do not select it out of the DC first by selecting the previous bitmap (obtained by your first call to SelectObject()). This will cause a memory leak and sometimes the debugger won't even know where it's at.

These leaks look like they are memory data blocks for some object, but I cannot tell.

> Any way to determine where these are coming from
Are the addresses consistent?
If they are, a 'breakpoint on a value' on the return address of the allocator may tell you who allocated it.

I'm sorry, Bubba, this is gonna sound a bit harsh, but your philosophy on Destroy vs. destructors is pretty much just wrong. Everytime you need to manually "Destroy" an object, it's a potential bugs. Most modern languages have garbage collection for just this reason.

I think this is why MS decided on Release() for COM objects. You can tell exactly when the object is cleaned up by simply looking at the code.

until you forget to call release, until something throws an exception, until some junior programmer sticks a return in before the release. This is why COM has not one but TWO handle classes, CComPtr and com_ptr_t. Release is a notorious source of bugs in COM.

C++ doesn't have gc. Instead it provides you with a fantastically powerful idiom: RAII. Your example of deleting a DC with a Bitmap selected is a prime candidate. Let's say you have the following code.

Code:

CDC *pDC = new CPaintDC();

CBitmap* pOld = pDC->SelectObject(pSomeBitmap);

// do stuff

pDC->SelectObject(pOld);
delete pDC;

and everything's cool, right?

until "// do stuff" throws an exception

on the other hand we can use RAII

Code:

class DCHandle
{
public:
   DCHandle()
   : m_pDC(newCDC)
   , m_pOldBitmap(NULL) {}
 
   ~DCHandle()
   {
       m_pDC->SelectObject(m_pOldBitmap);
   }

   void SelectObject(CBitmap* pB)
   {
       m_pOldBitmap = m_pDC->SelectObject(pB);
   }

    CDC *operator->(void)
    {
        return m_pDC.get();
    }

   
private:
   auto_ptr<CDC> m_pDC;
   CBitmap* m_pOldBitmap;
};

void OnPaint()
{
    DCHandle pDC;

    pDC.SelectObject(pSomeBitmap);

    // don't need to remeber to delete
    // if someone accidentally inserts a return or throws an exception
    // it's guarenteed to clean up
}

obviously, that's rough and can be cleaned up using generic handles (boost.shared_ptr and bind can really help)

I see but most frameworks I've seen do not rely on destructors for cleanup. They have a specific shutdown or destroy function that cleans everything up. So far this has worked quite well.
I dug into the MFC code for most of the idea and inside of the DirectX framework provided by MS. Perhaps it's just a design issue with MS, but they do not seem to go for the cleanup inside of the destructor approach.

Memory leaks like the ones you are talking about can be cleaned up in the release version. Nothing should throw an exception between those blocks if you've coded everything right.
However, if it does, then you can use try and catch as a more efficient means to catch, cleanup, and possibly recover. As far as COM leaks that is simply because the programmer has failed to call Release() which is akin to not calling delete in C++. So far I've had no problems with COM cleanup, but then Direct3D is not the most complicated COM out there either.

I'm using the boost library inside of the engine and the DirectX code due to all of the COM interface pointers, but decided not to use it inside of MFC. Perhaps a bad decision. I will, however, use auto_ptr for now.

Regardless of how you feel about the design I've not violated any OO principles. The object who allocates a resource is also resposible for cleaning it up. I do not rely on cross-module cleanups. So if class A allocates resource R and class B has a pointer to R, when B goes out of scope it does not touch R. Only when A goes out of scope is R cleaned up. The only major issue I have is that I do not have a mechanism to explicitly prevent accidental cleanup of R, outside of A.

Up until this version of the code all of my cleanup was inside of destructors. The 2005 compiler has a bad habit of compiling old code and NOT re-compiling your changes. This led to a lot of vector problems when it failed to compile my new version of the code which cleaned up the vector. A re-build fixed it since it recompiled the entire project. This has been quite confusing.
Lesson: If you alter your code, compile, run, and get the same errors - just rebuild the entire project and run it.

Seems to me this is a bug since MSVC 6 did not have this issue.
There are some other random bugs in the new compiler as well such as public, protected, and private being typed in for you on EVERY single variable instead of grouping them all into nice blocks.

I'll try what Salem suggested.

Also how much error checking should be in my release version? I cannot possibly do a try and catch on every single allocation attempt or access to an array. So what is the general rule of thumb for catching errors?

Originally Posted by Bubba

I see but most frameworks I've seen do not rely on destructors for cleanup. They have a specific shutdown or destroy function that cleans everything up.

Any C++ framework that relies on explicitly calling functions to cleanup is asking for trouble. Every used an ofstream? Do you have to call close() on that before it goes out of scope?

Originally Posted by Bubba

I dug into the MFC code for most of the idea and inside of the DirectX framework provided by MS. Perhaps it's just a design issue with MS, but they do not seem to go for the cleanup inside of the destructor approach.

First up, MFC is not generally held up as very well designed library in C++ circles, but even so it follows many of the principles I mentioned.

For example, have you looked at CDialog's destructor?

Code:

CDialog::~CDialog()
{
	if (m_hWnd != NULL)
	{
		TRACE(traceAppMsg, 0, "Warning: calling DestroyWindow in CDialog::~CDialog --\n");
		TRACE(traceAppMsg, 0, "\tOnDestroy or PostNcDestroy in derived class will not be called.\n");

		DestroyWindow();
	}
}

Notice that DestroyWindow is called here. This allows for neater code like the following

Code:

void someFunc(void)
{
    CMyDialog dlg; // derived from CDialog as normal

    // shows the dialog and cleans up as normal
    if (dlg.DoModal() == IDOK)
    {
         // do some stuff
    }
}

otherwise you'd have to write

Code:

void someFunc(void)
{
    CMyDialog dlg; // derived from CDialog as normal
    try
    {
        
        if (dlg.DoModal() == IDOK)
        {
            // do some stuff

            // and this STILL doesn't protect you if someone accidentally puts a return in here
        }
    }
    catch(...)
    {
         dlg.DestroyWindow();
    }
    
}

Originally Posted by Bubba

Memory leaks like the ones you are talking about can be cleaned up in the release version. Nothing should throw an exception between those blocks if you've coded everything right.

oh really? So you can guarentee that none of your code, framework code you call, or runtime code will throw an exception. Exceptions happen.

Originally Posted by Bubba

However, if it does, then you can use try and catch as a more efficient means to catch, cleanup, and possibly recover.

Try and catch is not a more efficient means of catch, cleanup and destroy. And it still doesn't protect you from accidental return statements.

Look in java, you have to write ugly code like this.

Code:

{
    File f = new file;
    try {
       f.doStuff();
    }
    finally {
       f.closeFile();
    }
}

but C++ provides deterministic destruction (basically scope), which is much cleaner.

Code:

{
    ofstream f;
    f.doStuff();
    // f goes out of scope here and is automatically cleaned up
}

Python has a similar scheme to Java, but even they've realised how powerful RAII is and are adding it to the next version of python.
Python "With" proposal

Originally Posted by Bubba

As far as COM leaks that is simply because the programmer has failed to call Release() which is akin to not calling delete in C++.

That's my point! The problem is that as long as machines are programmed by humans, mistakes like that will be made! So automate it, that's the whole point of programming in the first place!

instead of

Code:

void someFunc()
{
    IBlah* pBlah = GetSomeCOMPtr();
  
    // some long and complicated code


    // repeat 200 times: I must remember to release

    pBlah->Release();
}

// we can do this!
void someFunc()
{
    CComPtr<IBlah> pBlah(GetSomeCOMPtr());
  
    // some long and complicated code

    // I now don't care what happens here
    // go ahead, throw an exception, add a return
    // I'll still clean up and short of unplugging the machine 
    // there's nothing you can do to stop me!!
}

Originally Posted by Bubba

Regardless of how you feel about the design I've not violated any OO principles. The object who allocates a resource is also resposible for cleaning it up. I do not rely on cross-module cleanups. So if class A allocates resource R and class B has a pointer to R, when B goes out of scope it does not touch R. Only when A goes out of scope is R cleaned up.

That's not what you said earlier. You said you called A::Destroy.

Originally Posted by Bubba

Up until this version of the code all of my cleanup was inside of destructors. The 2005 compiler has a bad habit of compiling old code and NOT re-compiling your changes. This led to a lot of vector problems when it failed to compile my new version of the code which cleaned up the vector. A re-build fixed it since it recompiled the entire project. This has been quite confusing.
Lesson: If you alter your code, compile, run, and get the same errors - just rebuild the entire project and run it.

Ok that's a very worrying bug. I have to say I'm surprised at that. Most people have had good reports of (presumably Visual Studio?) 2005. I'd submit that as a bug to microsoft. 2003 certainly doesn't have that problem.

Originally Posted by Bubba

There are some other random bugs in the new compiler as well such as public, protected, and private being typed in for you on EVERY single variable instead of grouping them all into nice blocks.

Eh? I presume you're talking about MFC and the class wizard here. That has nothing to do with the compiler, that's the IDE. MS aren't really putting much effort into MFC anymore, as they want you to use .NET.

That's not what you said earlier. You said you called A::Destroy.

The assumption on that statement was that those objects were allocated by the main object. If A allocates B then A destroys B. If B allocates another object like C then B destroys C, not A. A would destroy B, which in turn would destroy C.

The main reason I did it this way for MFC code is that destructors are great for cleanup in C++, but are horrid inside of Windows. The actual Window object and thus it's Window DC and Client DC are no longer valid. Since you cannot access those w/o causing an access violation you must clean them up prior to the destructor.

Now you can clean up vectors, arrays, heap blocks, etc, etc inside of destructors but cleanup of window-based objects just does not work.

When Windows destroys the window or when MFC calls DestroyWindow() the object is no longer valid. Any future access to it's attributes will prove fatal to your code.

Also the destroy methods allow for a much cleaner cleanup process for instances in which the user wants to close the current document and open another one. Inside of that code I just call CDocument::Destroy() and CView::Destroy() which in turn cleans up all the objects and resources belonging to those classes. However note that in an SDI application if you call delete on CView or CDocument this will cause major problems. The document in SDI always exists even if it represents nothing. So this allows me to use the document object, yet change it's contents and in turn change the view since it only draws the view relative to the document data. If I was to delete these objects I would have to then re-allocate everything and pretty much restart the entire application because of the way MFC sets up the CWndApp, CFrameWnd, CDocument, CDocumentTemplate, and CView relationships.

So the destroy I reckon is more of an MFC-based idea for me than a C++ one. As I said the rest of my source code relies on destructors. The issue with access violations while attempting to access a windows or API object after Windows has destroyed it is what led me to use the Destroy() method.

Code:

CDialog::~CDialog()
{
	if (m_hWnd != NULL)
	{
		TRACE(traceAppMsg, 0, "Warning: calling DestroyWindow in CDialog::~CDialog --\n");
		TRACE(traceAppMsg, 0, "\tOnDestroy or PostNcDestroy in derived class will not be called.\n");

		DestroyWindow();
	}
}

This is only the CDialog class and this destructor cleanup method is not persistent throughout MFC. Some Windows objects are destroyed prior to the destructor and must be in order for the framework to function correctly. CDialog is a simple class and is not a good representation of the complexity that can be present in large applications utilizing MFC, MFC/Direct3D, and/or MFC/Win32/ATL,etc, etc.

Creation of objects also suffer this same fate. Most windows objects are not created until a Create() function or equivalent is called. Attempting to select objects into DC's, retrieving client rect's etc, inside of a constructor will throw an exception. So my constructors only intialize everything either to a NULL state (for pointers) or set initial values. None of them allocate or attempt to mess with resources.

I'm not sure I understand you argument as far as Java goes. Are you saying 'do it automatically' in relation to hidden destructor calls in C++ or are you referring to Java's garbage collector? If it's in relation to the garbage collector then we have a major debate because I'm not in favor of garbage collectors at all.

1. They constantly leak memory until they see it is fit to clean it up.
2. They only manage memory as it relates to the current application and take no thought for other resources on the system. The garbage collector might be working fine for the app, but system-wide it could be causing issues.
3. In C++ if you delete an object 100 times, the destructor will be called 100 times. In GC-based languages this is not true and has been shown that out of 100 times, the memory is cleaned up once at a time of it's choosing. This means most of the time it was taking up memory that was not needed by the app. Not a good system IMO.

And no I cannot guarantee that code between point A and point B will not fire off an exception. However, how much error checking is too much? Try and catch and other types of error checking clutter the code to the point of not being readable. I'm trying to find a balance between overkill and underkill.

For instance when you do this:

Code:

CDC *pDC=new CDC();
CBitmap *pBitmap=new CBitmap();

pDC->CreateCompatibleDC(GetDC());
pBitmap->CreateCompatibleBitmap(GetDC(),width,height);

CBitmap *pPrevBitmap=pDC->SelectObject(pBitmap);

pDC->DrawText(0,0,L"Testing 1,2,3");

pDC->SelectObject(&pPrevBitmap);

pBitmap->Detach();
pDC->Detach();

delete pBitmap;
delete pDC;

Every single line of this code could fail. Do I check every single line and every single return value?

Eh? I presume you're talking about MFC and the class wizard
here. That has nothing to do with the compiler, that's the IDE. MS aren't really putting much effort into MFC anymore, as they want you to use .NET.

Yet they have updated MFC to be compatible with GDI+ and added several new features to it. Obviously they realize not everyone is just going to switch to .NET if MFC is working just fine for them. Some companies don't like switching to some new thing just because it's the latest and greatest, at the risk of breaking their huge repository of code that already works just fine.

ok a quick response 'cos I'm really busy today.

Originally Posted by Bubba

stuff about mfc, cleanup, destroy and destructors

Ok I'll grant you that MFC does make it difficult. This is one of the reasons MFC is not a well designed modern C++ library.

Originally Posted by Bubba

I'm not sure I understand you argument as far as Java goes. Are you saying 'do it automatically' in relation to hidden destructor calls in C++ or are you referring to Java's garbage collector?

I was referring to Java's lack of destructors. It means that even when java eventually gc's an object, there's no way of providing automatic cleanup.

Now, it's been a whil since I've done any java, so feel free to correct me, but AFAIK, to open a file in java you need to do

Code:

	File inputFile = new File("farrago.txt");
	File outputFile = new File("outagain.txt");

        FileWriter out = new FileWriter(outputFile);

        out.write("something");

	// this line is unneccessary in C++
        out.close();

If it's in relation to the garbage collector then we have a major debate because I'm not in favor of garbage collectors at all.

I sort of semi-agree with you there. (vague enough? )

I love C++'s deterministic destruction. It's a massively powerful idiom IMO. But sometimes, I just want to create a new object, pass it around without copying it and then let it die. My ideal language would have gc AND deterministic destruction. kind of a paradox I guess.

And no I cannot guarantee that code between point A and point B will not fire off an exception. However, how much error checking is too much?

this was kinda my point about RAII. It allows you to throw exceptions or have errors and still cleanup without explicitly checking everything. Read up on the basic guarentee, the strong guarentee and the nothrow guarentee..

Yet they have updated MFC to be compatible with GDI+ and added several new features to it. Obviously they realize not everyone is just going to switch to .NET if MFC is working just fine for them. Some companies don't like switching to some new thing just because it's the latest and greatest, at the risk of breaking their huge repository of code that already works just fine.

1. I wasn't advocating a switch to .NET (although you'd be insane to start a new project in MFC these days).
2. MS may have added a few simple features (GDI+ probably took a week or two to add, hell you could hack it into an existing mfc app without much effort years ago), but despite what their marketing hype says, MFC is a dying technology in the eyes of developers and rightly so. Personnally, as soon as Boost.GUI is available I won't touch anything else

Dumping objects ->
{1878} normal block at 0x00DD2C98, 4 bytes long.
Data: < > A0 8E DE 00
{1875} normal block at 0x00DE8E48, 24 bytes long.
Data: < |C , > A8 7C 43 00 00 00 00 00 CD CD CD CD 98 2C DD 00
Object dump complete.

The number in braces is your allocation number. 1878 and then a 1875. You can break on a certain allocation like so:

Code:

_CrtSetBreakAlloc(1878);

However, if you have the time, one method to allow you to double click on the leak and have it take you to the offending allocation is:

http://logos.cs.uic.edu/Examples%20A...in%20VC++.html

But you have to put that in each cpp file, so kind of a pain.