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:
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:
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.
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.
Quote from: hutch-- on December 20, 2014, 11:53:24 AMyou 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. (http://msdn.microsoft.com/en-us/library/windows/desktop/dd744766%28v=vs.85%29.aspx)
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.
Quote from: jj2007 on December 20, 2014, 02:00:38 PM
Quote from: hutch-- on December 20, 2014, 11:53:24 AMyou 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. (http://msdn.microsoft.com/en-us/library/windows/desktop/dd744766%28v=vs.85%29.aspx)
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?
Quote from: NoCforMe on December 20, 2014, 02:10:42 PMButbutbut .... 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 (http://blogs.msdn.com/b/oldnewthing/archive/2009/01/29/9382123.aspx) :icon_mrgreen:
Magic rule[tm], allocate and deallocate in the same scope unless you make the handle global.
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 :biggrin:
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?
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
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 ...
Quote from: NoCforMe on December 20, 2014, 05:21:33 PMIt'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 :bgrin:
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
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.
Quote from: jj2007 on December 20, 2014, 07:56:43 PM
Quote from: NoCforMe on December 20, 2014, 05:21:33 PMIt'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 :bgrin:
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:
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.
Quote from: NoCforMe on December 22, 2014, 04:01:55 PM
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.
It reduces down to a person thinking that they know more about an operating system than those who created it. When an OS vendor documents a set of functions, its because they know how they work and when they recommend a set of procedures to clean up after allocating resources, it is so the OS remains stable and the application continues to run correctly without using up more resource than it needs.
It boils down to whether you every write anything that matters, if you do then you have little option than to be tidy in what you write cleaning up any mess you make on the fly in the correct scope. If what you write does not matter, then who cares. The only reason why a question and set of suggestions of this type are made is sheer lazyness, why do it the right way as specified by the OS vendor when you can be sloppy and write rubbish.
The combination of SelectObject() and DeleteObject() are done for a reason, behind the GDI functions is very high level code that is much more complex that simplistic assumptions make out. You select and object while saving the old object handle, use what you need then reselect the old object then delete the new one. This way the system is not being burdened by wasted resource and your app if its doing anything more than trivial remains stable.
Quote from: hutch-- on December 22, 2014, 07:42:16 PMThe combination of SelectObject() and DeleteObject() are done for a reason, behind the GDI functions...
Hutch,
There are no contradictions to what you are writing:
Quote from: adeyblue on December 22, 2014, 04:57:29 PMAll 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.
We all agree on that, and it's also pretty obvious that e.g. SetTextColor and SetBkColor don't fall into this category. Interesting adeyblue's point on regions btw.
What is less obvious is the habit of carefully destroying objects before throwing them into the dustbin of ExitProcess. The documentation is somewhat vague on this point, unfortunately. It would be in the good tradition of the Masm32 Forum to test it empirically, by launching a Million little apps and testing if
- fonts
- bitmaps
- system strings
- GlobalAllocs
- VirtualAllocs
- ...
all belong to the same category "gone once the process is gone", or whether some belong to the category "global resources, to be treated differently". This is badly documented by Microsoft, and my own experience tells me that, in contrast to the rule "process dead means all memory is released", it's not that simple. For example, badly crashing an app that has hundreds of megabytes heapalloc'ed may result in slowing down the system - which is illogical because Windows
should clean up the mess but... ::)
And again, the question is whether this is limited to processes that are ended "exceptionally" (pun intended), and whether the same can happen with ExitProcess, too (which I doubt, but damn, where is the documentation that treats this in non-ambiguous language??).
The same doubts I have on fonts, because I have seen occasionally that after heavy testing the whole system had difficulties with fonts. If Notepad uses SYSTEM_FIXED_FONT in menus, then something has gone wrong. It shouldn't, with an OS that claims to safely separate all applications since the days of NT, but it does happen.
It's not strictly necessary to return the DC back to its exact original state; the point is to not leave any allocated objects selected into the DC.
The DC is initialised with a set of stock objects (system allocated and persistent.) Each time you call SelectObject, the previous object of the same type is returned, and in the case of a fresh DC that will be a stock object (assuming your DC is not shared.)
So, as long as you leave the DC with a set of stock objects, there should be no problems. To do this, you have the option of simply restoring the objects returned by calls to SelectObject, or by retrieving stock objects of the same type with GetStockObject -- the result will be the same.
The reason the GDI is this way comes from the early versions of Windows, where DCs were a shared resource, and so you would have to leave a DC in the exact state you found it so that you wouldn't mess up another process' graphics. DCs are no longer shared by default, so there is no strict obligation to return the DC to its original state.
It is arguable that there's no need to even de-select objects from the DC (though it definitely was necessary up until Windows XP), but this is the same argument as "it's not necessary to close file handles because the OS will close them on exit." You should. The OS tries to clean up to cover for exceptions, where a process wouldn't have had the opportunity to clean up correctly itself; it is not an excuse to be lazy.
You can choose to write sloppy code because you think you're saving a few function calls (you're not, the OS will still have to call them on your behalf), but what is the benefit?
QuoteIt's not strictly necessary to return the DC back to its exact original state; the point is to not leave any allocated objects selected into the DC.
So, would something like this make y'all happier?
do_paint:
...
INVOKE SelectObject, hDC, OpOutputFontHandle
MOV prevFont, EAX
...
INVOKE TextOut, hDC, $opOutputOffsetX, $opOutputOffsetY, OFFSET OpString, EDX
INVOKE SelectObject, hDC, prevFont
INVOKE EndPaint, hWin, ADDR ps
XOR EAX, EAX
RET
i generally try to double-buffer
that makes a difference, because you are not modifying a DC related to a physical device
instead, you are modifying a memory DC
the other thing is - SaveDC and RestoreDC take out the headaches
this is an example - double-buffered using SaveDC/RestoreDC....
http://masm32.com/board/index.php?topic=3344.msg35250#msg35250 (http://masm32.com/board/index.php?topic=3344.msg35250#msg35250)
if you read that thread in it's entirety, you might find something useful