News:

MASM32 SDK Description, downloads and other helpful links
MASM32.com New Forum Link
masmforum WebSite

How to avoid leaky Windows programs

Started by NoCforMe, October 23, 2011, 03:00:17 AM

Previous topic - Next topic

NoCforMe

Quote from: jj2007 on October 23, 2011, 05:27:13 PM
Re bad programming habits: if there is a clear documentation saying ExitProcess does all the cleanup, then it is bad programming to do the same before ExitProcess.

At the very least it would seem to be redundant programming practice to do so ... why do something that's already done?

redskull

Quote from: NoCforMe on October 23, 2011, 07:49:16 PM
At the very least it would seem to be redundant programming practice to do so ... why do something that's already done?

So then I suppose putting airbags in cars absolves you of the responsibility of driving safely?   :wink 

The reason is that you don't know when, or even if, your program will ever end.  The user is well within his rights to leave it running all day, every day, which means all the resources you are sucking up are both unavailable to other programs, as well as hurting your quotas.  Your program should use what it needs, when it needs it, and nothing else.  Imagine the nightmare if every program grabbed all the resources it thinks it might ever need, and never let them go...

-r
Strange women, lying in ponds, distributing swords, is no basis for a system of government

NoCforMe

Hmm, point taken. I would then amend my approach to say that one should release  any significant resources that are no longer needed (mainly large chunks of memory). I still wouldn't worry much about a few handles to objects. Now, if your program were to use hundreds or thousands of such objects, that might be a different story, and it might be a Good Idea to release the unused ones.

hutch--

Roughly, catchalls, error handlers for non critical hardware tasks and the like are just sloppy programming. Our friend may like the idea of NO C FOR ME but with an operating system based on C and later C++ you either do it the right way or end up writing crap that is unreliable. Really what some of you guys need is to write 16 bit Windows code where one mistake took the OS and every other running app down.

Memory allocation, high level resource usage and the like have de-allocation routines for a reason, they are not there to just to be  ignored. The simple test is to run task manager and if your memory usage keeps going up, you have a leak from failing to release a resource or de-allocate memory.

At the stage of ExitProcess() your application should have the same resource/memory usage that it had at startup, if not you need to fix it.
Download site for MASM32      New MASM Forum
https://masm32.com          https://masm32.com/board/index.php

fearless

Quote from: hutch-- on October 23, 2011, 09:21:19 PMThe simple test is to run task manager and if your memory usage keeps going up, you have a leak from failing to release a resource or de-allocate memory.

So true. That happened to me recently with my memory leak from STM_SETIMAGE - http://www.masm32.com/board/index.php?topic=17270.msg144769#msg144769 and i used task manager to identify if i had an issue.
ƒearless

redskull

Quote from: hutch-- on October 23, 2011, 09:21:19 PM
Really what some of you guys need is to write 16 bit Windows code where one mistake took the OS and every other running app down.

Did you walk uphill through the snow, both ways, to get to that Win16 box?  :toothy

But in response to the suggestion that only "significant" resources get deallocated, who determines that?  If your program occasionally uses a file, is it okay to keep that object allocated, locking every other program out from using it for the duration?  If your program occasionally listens on port 80, is it okay to usurp that socket from any other application?  There are more resources than just memory, and more significance than just size.

-r
Strange women, lying in ponds, distributing swords, is no basis for a system of government

hutch--

 :bg

Red,

Just about, something these young fellas missed out on was software simulated multitasking where all of Windows was one big unhappy app, one blunder and you bypassed the BLUE screen of death and went directly to the BLACK screen of death that did not even have text on it. One learnt GlobalFree() by heart in those days, you got a little time with forgetting to free a brush before it all locked up and the OS stopped.

On the UP side, Windows 3.?? used to boot much faster than the later 32 bit versions so it was not as big a deal.
Download site for MASM32      New MASM Forum
https://masm32.com          https://masm32.com/board/index.php

dedndave

Quote...the BLACK screen of death that did not even have text on it
i remember those   :bg
if you were lucky, you had a blinking cursor in the upper left hand corner

Gunner

If you allocate memory, create a font, brush, gdi object, or other system resource.  Just free/delete it when done, that is good programming practice.
~Rob (Gunner)
- IE Zone Editor
- Gunners File Type Editor
http://www.gunnerinc.com

MichaelW

#24
I recall reading in the Microsoft documentation about limited resources that you "must" release when you finish using then. ATM the only one that comes to mind is common DCs:

http://msdn.microsoft.com/en-us/library/dd183571(v=VS.85).aspx

So I decided do an experiment to see what the effects of not releasing a large number of common DCs would be. My test code:

;==============================================================================
include \masm32\include\masm32rt.inc
;==============================================================================
    .data
        hwndStatic  dd 0
        hFile       dd 0
        dcCount     dd 0
    .code
;==============================================================================

DialogProc proc hwndDlg:DWORD, uMsg:DWORD, wParam:DWORD, lParam:DWORD

    SWITCH uMsg

        CASE WM_INITDIALOG
       
            mov hFile, fopen("testlog.txt")
            .IF hFile == INVALID_HANDLE_VALUE
                  mov hFile, fcreate("testlog.txt")
            .ENDIF
            mov eax, fseek(hFile,0,END)

            invoke GetTickCount
            push eax

            ID=1000
            REPEAT 1024

                invoke GetDlgItem, hwndDlg, ID
                mov hwndStatic, eax
                .IF eax == 0
                    fprintc hFile, chr$("GetDlgItem failed")
                    jmp @F
                .ENDIF

                ;-----------------------------------------------------
                ; Ensure that GetDC will return a common DC for the
                ; static control, instead of a private DC as it would
                ; be if the control had the CS_OWNDC class style.
                ;-----------------------------------------------------

                invoke GetClassLong, hwndStatic, GCL_STYLE
                .IF eax & CS_OWNDC
                    fprintc hFile, chr$("CS_OWNDC")
                    jmp @F
                .ENDIF

                invoke GetDC, hwndStatic
                .IF eax == NULL
                    fprintc hFile, cfm$("GetDC failed\n")
                    jmp @F
                .ENDIF

                inc dcCount
                ID=ID+1

            ENDM

          @@:

            fprintc hFile, sdword$(dcCount)
            fprintc hFile, chr$(9)
            invoke GetTickCount
            pop edx
            sub eax, edx
            fprintc hFile, sdword$(eax)
            fprintc hFile, chr$(13,10)

            fclose hFile

        CASE WM_COMMAND

            SWITCH wParam

                CASE IDCANCEL

                    invoke EndDialog, hwndDlg, NULL

            ENDSW

        CASE WM_CLOSE

            invoke EndDialog, hwndDlg, NULL
    ENDSW

    return 0

DialogProc endp

;==============================================================================
start:
;==============================================================================

    Dialog  "Test", \
            "MS Sans Serif",10, \
            WS_OVERLAPPED or WS_SYSMENU or DS_CENTER, \
            1024, \
            0,0,112,122, \
            1024*32

    X=0
    Y=0
    ID=1000
    REPEAT 32
        REPEAT 32
            DlgStatic 0,WS_BORDER,X,Y,2,2,ID
            X=X+3
            ID=ID+1
        ENDM
        X=0
        Y=Y+3
    ENDM

    invoke GetModuleHandle, NULL
    CallModalDialog eax, 0, DialogProc, NULL
    exit
;==============================================================================
end start


To get representative times in the log file you must wait for the dialog to be fully displayed before you start another instance of the app.

And here is the log file for a test under Windows 2000 where I continued starting new instances until there was obviously a problem.

1024 330
1024 802
1024 1402
1024 2113
1024 2884
1024 3695
1024 4517
1024 5328
1024 6129
1024 6910
1024 7681
1024 8482
1024 9294
GetDC failed
607 5908


Somewhere around the point where GetDC failed, and at ~13K unreleased common DCs, the desktop stopped refreshing correctly. Surprising to me, after I closed all of the running instances everything returned to normal, as near as I can tell by just using the system.

Edit: I forgot to add that I had run the app something like 100 times prior to the test that failed, without any sign of a problem, and I had previously had more than 20 instances running, but at that point I was still chasing bugs so I was building it as a console app.

Edit2: It doesn't always fail at the same point. Here are the results for another trial:

1024 271
1024 701
1024 1272
1024 1923
1024 2674
1024 3655
1024 4537
1024 5518
1024 6780
1024 7981
1024 8543
1024 9624
1024 10705
1024 12388
GetDC failed
157 2354


And again, everything seems to have returned to normal after I closed all instances of the app.

eschew obfuscation

jj2007

Quote from: MichaelW on October 24, 2011, 05:45:21 AM
So I decided to do an experiment
:U

Quote
Somewhere around the point where GetDC failed, and at ~13K unreleased common DCs, the desktop stopped refreshing correctly. Surprising to me, after I closed all of the running instances everything returned to normal, as near as I can tell by just using the system.

There are 4 aspects that need to be kept apart here:

1. What to do while the app is running: As Michael shows, don't worry, you can have 13,000 dialogs open, in parallel - imagine your taskbar :bg Of course, nobody stops you from creating 1,000 times one and the same font or brush, and deleting it at the end of the WM_PAINT handler; some might call it bad programming practice, though.

2. What to do with a really fat memory allocation (>500 MB) while the app is running: If you don't need them any more, you might release them. However, keeping it should not be a problem, since either your app is active (then others don't need the mem unless they do funny things in background), or another app is active, and if the other one needs the memory, the OS will write your memory to the swap file.

3. What to do with memory, fonts, DCs etc before calling ExitProcess: Nothing, folks, the OS frees everything for you.

4. What to do with memory, fonts, DCs etc before the app is crashing with exception C0000000h: Chase the bug!

jj2007

Just found one interesting exception on MSDN:

QuoteNo window classes registered by a DLL are unregistered when the DLL is unloaded. A DLL must explicitly unregister its classes when it is unloaded.

I wonder if it applies to other resources, too.
For the freaks, there is a really interesting article here by Denis Mikhalkin, "Debugging USER object leak"

Tedd

I'm not sure why this is even an issue - if you allocate it, free it when you've finished with it.
It's not a difficult concept, it's not complex to implement, just clean up after yourself ::)
Yes, you may be able to get away with not freeing some things sometimes, on the expectation that it 'should' be freed for you on exit, but you don't know how, when, or even if that will happen. And for the sake of avoiding a few extra function calls, what's the benefit?

How to avoid leaky Windows programs: clean up after yourself!
If you open/create files - close them; if you allocate memory - free it; if you obtain handles to graphical objects - free them; stock objects don't need to be freed by you since they weren't allocated by you.
No snowflake in an avalanche feels responsible.

ixuta

Quote from: hutch-- on October 23, 2011, 09:21:19 PM
...write 16 bit Windows code where one mistake took the OS and every other running app down.

Ah the good old days... :green  reminds me of the day OS/2 first went on sale and I took it down right in front of the rep who was trying to sell it to me! Oh, the shock on his face when OS/2 locked up. :boohoo:

Anyway, this is a good thread, I've looked about for a way to ""Pin this thread to my favorites"", but haven't found a way to do that  ... is there?