Easy way to destroy stuff in a program

[NoCforMe]

What I'm talking about is destroying unneeded stuff at program termination--you know, all those brushes, pens, bitmaps, etc., etc., that you forgot to get rid of, and now you're getting memory leaks ...

Not that I ever got any perceptible leaks from not doing this, but it occurred to me that rather than trying to remember every itty-bitty object created and then calling DeleteObject() for each and every one, there had to be an easier way. Which there is.

I can't believe this is original to me, though I haven't seen this anywhere else. What I did was create a routine you call any time you create something that's gonna need to be deleted later, which puts it on a list of stuff to nuke. Then at program exit, you just go down the list and delete everything:

Code Select Expand


Items2Destroy DD $maxDestroyableItems DUP (0)
DestroyPtr DD Items2Destroy
DestroyCount DW 0


;====================================================================
; Add2ListOfStuff2Destroy()
;
; Adds an item (handle in EAX) to a list of things to be destroyed
; on program termination.
;
; EAX is preserved on exit.
;====================================================================

Add2ListOfStuff2Destroy PROC

CMP DestroyCount, $maxDestroyableItems ;Reached our limit?
JNE a2l10
PUSH EAX
INVOKE MessageBox, NULL, OFFSET MaxDestroyableItemsMsg, NULL, MB_OK
POP EAX
RET

a2l10: MOV EDX, DestroyPtr
MOV [EDX], EAX
ADD DestroyPtr, 4
INC DestroyCount
RET

Add2ListOfStuff2Destroy ENDP


;====================================================================
; DestroyStuff()
;
; Destroys all items on destruction list at program termination.
;====================================================================

DestroyStuff PROC

MOVZX ECX, DestroyCount
JECXZ ds99
PUSH EBX
LEA EBX, Items2Destroy

ds10: PUSH ECX ;Save register across API call.
INVOKE DeleteObject, [EBX]
POP ECX
ADD EBX, 4 ;Next item.
LOOP ds10
POP EBX
ds99: RET

DestroyStuff ENDP


To use it, just stick a call to the add routine right after you create something needing destroyed:

Code Select Expand


INVOKE LoadBitmap, hInst, BitmapID
MOV BitmapHandle, EAX
CALL Add2ListOfStuff2Destroy


Simple and it works. Notice you don't even need to INVOKE it, since it has no parameters. You could add refinements: better error checking if the list is full, dynamic memory allocation if you need to get rid of lots of items.

[hutch--]

Its before exit during runtime that you can have problems if you don't write carefully balanced code in terms of cleaning up stray memory allocations, this is in fact what is happening with GDI leaks. If you are using large image data on a high repeat rate, you can exhaust available memory while the app is running and while the OS will recover it on app closure, you can crash an app by not cleaning up properly in the first place.

The magic rule is like the law of gravity, what goes up must come down, allocate and free memory in the same scope and you have stopped living dangerously.

[jj2007]

you can exhaust available memory while the app is running and while the OS will recover it on app closure, you can crash an app

More precisely, Microsoft says that Windows will free all memory allocated by the application on process termination.

Which means that all attempts to be kind to the OS by freeing objects shortly before the WM_QUIT message are a waste of time and programming efforts. Handles sit in the process' address space, and that one says bye bye forever on ExitProcess.

The story is different during the lifetime of the app. If you allocate memory for a fat image every n seconds, and forget to free it when no longer needed, then you might as well call your application "Firefox" or similar. The problem with recent Windows versions is that you may not notice such kind of leaks - there is so much memory available that wasting a few hundred megabytes here and there may go unnoticed. During testing, that is: If the dumb end user leaves the app running for weeks, yes then it may crash the system.

A word of caution on releasing handles: Your code makes sure that every font etc gets recorded, so that afterwards you can call DestroyWhatever and it won't be forgotten. Great. However, DestroyWhatever can fail miserably if, for example, the font is still selected in a device context. Even worse, if other parts of your app rely on objects that you created and then "automatically" destroyed, then your app may crash. "May" implying that it could also continue using the object for a while because it is still hanging around in memory, so during the test phase everything is working fine, but wait until the dumb end user etc etc - you get the concept.

[NoCforMe]

you can exhaust available memory while the app is running and while the OS will recover it on app closure, you can crash an app

More precisely, Microsoft says that Windows will free all memory allocated by the application on process termination.

Butbutbut .... then why the hell do we need to run around deallocating stuff at program's end like we've been told we're supposed to do? (Perhaps that's due to widespread misunderstanding ...)

I read the linked article, but it applies to Windows 7. Do previous versions also clean up nicely after themselves?

So it might be more important to design a more robust DestroyStuff() for use during program execution, yes?

Another question: at what point can things like bitmaps and brushes be deleted? Say I create a brush to pass to RegisterClass() to set the window background color; can I get rid of it once that call completes? Does the OS copy and store it somewhere? Or do I need to keep it around until program termination?

[jj2007]

Butbutbut .... then why the hell do we need to run around deallocating stuff at program's end like we've been told we're supposed to do? (Perhaps that's due to widespread misunderstanding ...)

I read the linked article, but it applies to Windows 7. Do previous versions also clean up nicely after themselves?

All you have to do is read this clear and concise documentation :icon_mrgreen:

[hutch--]

Magic rule[tm], allocate and deallocate in the same scope unless you make the handle global.

[dedndave]

i try to keep things balanced, as Hutch mentioned
and - it may not be completely necessary

where problems arise is when you select an object into a DC (or otherwise modify the state),
then delete the DC without returning it to it's original state

you can write a simple test piece - run it several times, and watch windows go bonkers on you until you restart   

[NoCforMe]

Now that I really don't understand, this business of retracing your steps to return a DC to its original pristine condition before releasing it. (Me, I just don't bother doing so; but then, I'm not grinding out commercial code day in and day out.)

I mean, what difference does it make? Say you get a DC, say in your WM_PAINT handler, and you change the text color because you want some text to be red; what does Windows care that you've altered it? It's just some kind of structure with stuff (brushes, pens, etc.) in it, right? When it gets deleted, into the bit bucket it goes. It's not as if there are any magical links inside there that have to be followed in case the user changes something, right?

Or am I missing something here?

[dedndave]

i'm with you - it doesn't make sense
i just know it causes trouble

i am trying to get into the habit of using SaveDC and RestoreDC, rather than saving/restoring all the original values   :t

[NoCforMe]

Isn't it possible that you're attributing problems to superstition, rather than what's actually causing them? Don't see how nothing but a modified DC could bend Windoze out of shape ...

[jj2007]

It's just some kind of structure with stuff (brushes, pens, etc.) in it, right? When it gets deleted, into the bit bucket it goes.

Well, no, it's not that simple. The DC has a pointer to MyCurrentFont, and you ask it to delete MyCurrentFont. The OS has two options:
- don't delete the font, because otherwise the pointer is invalid, and the next attempt to use it causes an exception; so the font stays in memory, and if you do that in a loop, many new font structures accumulate
- delete the font, replace the pointer with one to ANSI_FIXED_FONT.

Both options will scare the dumb end user, so Microsoft kindly asks coders to follow the rules

[dedndave]

not knowing the inner workings of windows, i am left to guessing....

perhaps there is a handle usage count
you select a handle into a DC, and the count is incremented
you replace that handle, and the count is decremented

when you delete a DC without restoration, the count is left at non-zero
so - you delete the object - the count isn't 0 - the OS isn't able to delete it
bang - memory leak because the resources used for that object remain in use

[Tedd]

The idea is nice, but ultimately leads to laziness and bad habits -- "I won't bother freeing my objects, they'll just get cleaned up at the end anyway." Which sounds nice enough, but that ignores the fact they still stay allocated for the lifetime of your program (as others have mentioned.)
It is much better to allocate objects as needed, and free them once no longer needed; while some objects will be required for the program's lifetime, so it makes sense to allocate them once and free them at the end.

As for superstition, I learned the hard way that freeing objects which are still associated with a device context will leave them as orphans; after multiple runs, the system became unusable because the orphans had used up available memory, and couldn't be freed.
Luckily, the Windows' GDI has changed somewhat in the past decade, so it's much harder to cause leaks than it used to be. But that is no excuse for sloppiness. If you allocate something, acquire something, open a file, etc., then you should obviously release it when you're done - that's just good practice.

One way the idea could be useful, though, is instead of silently freeing orphaned objects, it could display a list of what objects remain un-freed upon program exit. You could also add in information about where the allocation was made, to make it easier to track. This would then allow you to tidy up your allocation/freeing during development so you can be confident there are no leaks in the final release.

[NoCforMe]

It's just some kind of structure with stuff (brushes, pens, etc.) in it, right? When it gets deleted, into the bit bucket it goes.

Well, no, it's not that simple. The DC has a pointer to MyCurrentFont, and you ask it to delete MyCurrentFont. The OS has two options:
- don't delete the font, because otherwise the pointer is invalid, and the next attempt to use it causes an exception; so the font stays in memory, and if you do that in a loop, many new font structures accumulate
- delete the font, replace the pointer with one to ANSI_FIXED_FONT.

Both options will scare the dumb end user, so Microsoft kindly asks coders to follow the rules

No, no: of course what you describe is a recipe for disaster, but that's not what I mean.

What I'm talking about is the business of returning the DC to its original state before discarding it, which I propose is unnecessary and somewhat silly. Take this example:

Code Select Expand


do_paint:
INVOKE BeginPaint, hWin, ADDR ps
MOV hDC, EAX
INVOKE SetTextAlign, EAX, TA_BASELINE OR TA_CENTER OR TA_NOUPDATECP
INVOKE SetBkColor, hDC, $colorWhite
INVOKE SelectObject, hDC, OpOutputFontHandle
INVOKE SetTextColor, hDC, $colorRed
LEA EAX, OpString
CALL strlen ;Get length in EDX.
INVOKE TextOut, hDC, $opOutputOffsetX, $opOutputOffsetY, OFFSET OpString, EDX
INVOKE EndPaint, hWin, ADDR ps
XOR EAX, EAX
RET

Nothing that's being used by the DC is being prematurely deleted here. The point is that I'm changing several attributes of the DC--background color, font and text color--before discarding it (via EndPaint() ). I fail to see why I should restore these things before doing so; what difference does it make? It's not as if I'm deleting things (handles) that are needed later; I'm simply altering the DC before using it, then discarding it. What harm could that cause? Once Windows releases the DC, it's a dead matter, no longer active, and presumably properly de-allocated.

I'm open to being persuaded differently, but at this point it seems simply like programmer's paranoia or superstition, handed down from coder to coder through the years, without any real substantiation.

Just to be clear, I do understand why what you described would be a bad thing, but that's different from what I'm describing here.

[adeyblue]

I fail to see why I should restore these things before doing so; what difference does it make? It's not as if I'm deleting things (handles) that are needed later; I'm simply altering the DC before using it, then discarding it. What harm could that cause?

None, and you don't have to do it. All you need to reset is the stuff returned from SelectObject (except regions), for the reasons outlined above. At least, all these GDI docs were written at the same time, and the only one that mentions it is SelectObject. The other functions returning the previous thing in the DC do so probably to keep the API behaviour consistent, but you're under no obligation to put them back.