Author Topic: Is this a leak?  (Read 1133 times)

BlameTroi

  • Regular Member
  • *
  • Posts: 11
  • Blame me, everyone does
Is this a leak?
« on: January 14, 2017, 12:01:56 PM »
If so, it's probably minor, but I want to make sure I'm paying proper attention to memory.

I'm working through Iczelion's tutorials and in tutorial 5 a font is created and used but never deleted. I think a DeleteObject call should be inserted.

Code: [Select]
invoke CreateFont,24,16,0,0,400,0,0,0,OEM_CHARSET,\
                            OUT_DEFAULT_PRECIS,CLIP_DEFAULT_PRECIS,\
                            DEFAULT_QUALITY,DEFAULT_PITCH or FF_SCRIPT,\
                            ADDR FontName
invoke SelectObject, hdc, eax
mov    hfont,eax
RGB    200,200,50
invoke SetTextColor,hdc,eax
RGB    0,0,255
invoke SetBkColor,hdc,eax
invoke TextOut,hdc,0,0,ADDR TestString,SIZEOF TestString
invoke SelectObject,hdc, hfont
invoke DeleteObject,eax ; <------ add this line after restoring old font
"God made the integers, all else is the work of man." -- Leopold Kronecker.

HSE

  • Member
  • ****
  • Posts: 551
  • <AMD>< 7-32>
Re: Is this a leak?
« Reply #1 on: January 14, 2017, 12:34:07 PM »
Perfect  :t

If you have ObjAsm32 installed, you can use Resguard to detect leaks:

Code: [Select]
.386
.model flat,stdcall
option casemap:none
WinMain proto :DWORD,:DWORD,:DWORD,:DWORD

include \masm32\include\windows.inc
include \masm32\include\user32.inc
include \masm32\include\kernel32.inc
include \masm32\include\gdi32.inc
includelib \masm32\lib\user32.lib
includelib \masm32\lib\kernel32.lib
includelib \masm32\lib\gdi32.lib

include \Masm32\Include\ResGuard.inc        ; <-----------  HERE     
includelib \Masm32\Lib\ResGuard.lib

RGB macro red,green,blue
        xor eax,eax
        mov ah,blue
        shl eax,8
        mov ah,green
        mov al,red
endm

.data
ClassName db "SimpleWinClass",0
AppName  db "Our First Window",0
TestString  db "Win32 assembly is great and easy!",0
FontName db "script",0

.data?
hInstance HINSTANCE ?
CommandLine LPSTR ?

.code
 start:
    invoke GetModuleHandle, NULL
    mov    hInstance,eax
    invoke GetCommandLine
    mov CommandLine,eax

    invoke ResGuardStart        ;        <-------  HERE

    invoke WinMain, hInstance,NULL,CommandLine, SW_SHOWDEFAULT

    invoke ResGuardShow         ;        <-------  HERE

    invoke ExitProcess,eax


WinMain proc hInst:HINSTANCE,hPrevInst:HINSTANCE,CmdLine:LPSTR,CmdShow:DWORD
    LOCAL wc:WNDCLASSEX
    LOCAL msg:MSG
    LOCAL hwnd:HWND
    mov   wc.cbSize,SIZEOF WNDCLASSEX
    mov   wc.style, CS_HREDRAW or CS_VREDRAW
    mov   wc.lpfnWndProc, OFFSET WndProc
    mov   wc.cbClsExtra,NULL
    mov   wc.cbWndExtra,NULL
    push  hInst
    pop   wc.hInstance
    mov   wc.hbrBackground,COLOR_WINDOW+1
    mov   wc.lpszMenuName,NULL
    mov   wc.lpszClassName,OFFSET ClassName
    invoke LoadIcon,NULL,IDI_APPLICATION
    mov   wc.hIcon,eax
    mov   wc.hIconSm,eax
    invoke LoadCursor,NULL,IDC_ARROW
    mov   wc.hCursor,eax
    invoke RegisterClassEx, addr wc
    invoke CreateWindowEx,NULL,ADDR ClassName,ADDR AppName,\
           WS_OVERLAPPEDWINDOW,CW_USEDEFAULT,\
           CW_USEDEFAULT,CW_USEDEFAULT,CW_USEDEFAULT,NULL,NULL,\
           hInst,NULL
    mov   hwnd,eax
    invoke ShowWindow, hwnd,SW_SHOWNORMAL
    invoke UpdateWindow, hwnd
    .WHILE TRUE
                invoke GetMessage, ADDR msg,NULL,0,0
                .BREAK .IF (!eax)
                invoke TranslateMessage, ADDR msg
                invoke DispatchMessage, ADDR msg
    .ENDW
    mov     eax,msg.wParam
    ret
WinMain endp

WndProc proc hWnd:HWND, uMsg:UINT, wParam:WPARAM, lParam:LPARAM
    LOCAL hdc:HDC
    LOCAL ps:PAINTSTRUCT
    LOCAL hfont:HFONT

    .IF uMsg==WM_DESTROY
        invoke PostQuitMessage,NULL
    .ELSEIF uMsg==WM_PAINT
        invoke BeginPaint,hWnd, ADDR ps
        mov    hdc,eax
        invoke CreateFont,24,16,0,0,400,0,0,0,OEM_CHARSET,\
                                       OUT_DEFAULT_PRECIS,CLIP_DEFAULT_PRECIS,\
                                       DEFAULT_QUALITY,DEFAULT_PITCH or FF_SCRIPT,\
                                       ADDR FontName
        invoke SelectObject, hdc, eax
        mov    hfont,eax
        RGB    200,200,50
        invoke SetTextColor,hdc,eax
        RGB    0,0,255
        invoke SetBkColor,hdc,eax
        invoke TextOut,hdc,0,0,ADDR TestString,SIZEOF TestString
        invoke SelectObject,hdc, hfont
        invoke EndPaint,hWnd, ADDR ps
    .ELSE
        invoke DefWindowProc,hWnd,uMsg,wParam,lParam
        ret
    .ENDIF
    xor    eax,eax
    ret
WndProc endp

end start

jj2007

  • Member
  • *****
  • Posts: 7730
  • Assembler is fun ;-)
    • MasmBasic
Re: Is this a leak?
« Reply #2 on: January 14, 2017, 12:55:18 PM »
in tutorial 5 a font is created and used but never deleted. I think a DeleteObject call should be inserted.

Correct in this case, as it is in a WM_PAINT handler, i.e. it may get created very often.

However, if you create the font once in the WM_CREATE handler, and move the handle to a global variable, you can use it as often as you want, and there is no need to delete it in the WM_DESTROY handler. Apart from very few exceptions, all "resources" exist only in the process space, which gets cleaned completely by ExitProcess.

BlameTroi

  • Regular Member
  • *
  • Posts: 11
  • Blame me, everyone does
Re: Is this a leak?
« Reply #3 on: January 14, 2017, 01:23:50 PM »
... you can use it as often as you want, and there is no need to delete it in the WM_DESTROY handler. Apart from very few exceptions, all "resources" exist only in the process space, which gets cleaned completely by ExitProcess.

Good to know but I'm a bit OCD from past experience, so I'll probably make a concerted effort to clean up after myself. ObjAsm32 mentioned by HSE can help me check myself.
"God made the integers, all else is the work of man." -- Leopold Kronecker.

hutch--

  • Administrator
  • Member
  • ******
  • Posts: 4922
  • Mnemonic Driven API Grinder
    • The MASM32 SDK
Re: Is this a leak?
« Reply #4 on: January 14, 2017, 02:20:25 PM »
Troi,

It sounds like you need some reference material. If you can get it, the old WIN32.HLP is a good reference for the main core Windows API functions. You can also plow through MSDN but it can be slow going at times. Its a good habit to tidy up after you do anything that allocates memory, it this case GDI memory so if you look up the function, it will tell you if it needs to be deallocated after.
hutch at movsd dot com
http://www.masm32.com    :biggrin:  :biggrin:

BlameTroi

  • Regular Member
  • *
  • Posts: 11
  • Blame me, everyone does
Re: Is this a leak?
« Reply #5 on: January 15, 2017, 12:33:42 AM »
Hutch, I already have all that but I am still getting used to finding my way around in it. I used quite a few Win32 API calls from .NET, but it's been 6+ years. Right now I'm still experiencing what I call tunnel vision that happens with any newish thing. My focus is too narrow at first but with familiarity it widens.
"God made the integers, all else is the work of man." -- Leopold Kronecker.

jj2007

  • Member
  • *****
  • Posts: 7730
  • Assembler is fun ;-)
    • MasmBasic
Re: Is this a leak?
« Reply #6 on: January 15, 2017, 03:15:40 AM »
if you look up the function, it will tell you if it needs to be deallocated after.

CreateFont:
Quote
When you finish with the CFont object created by the CreateFont function, use CDC::SelectObject to select a different font into the device context, then delete the CFont object that is no longer needed.

What MSDN doesn't tell you is that
a) you can keep the font for the lifetime of the application (i.e. create once in WM_CREATE, use many times in WM_PAINT)
b) it makes absolutely no sense to use DeleteObject shortly before ExitProcess.

If you are interested in the gory details, read Raymond Chen, Quick overview of how processes exit:
Quote
Note that I just refer to it as the way processes exit on Windows XP rather than saying that it is how process exit is designed. As one of my colleagues put it, "Using the word design to describe this is like using the term swimming pool to refer to a puddle in your garden."

Of course, we had that discussion already :P

HSE

  • Member
  • ****
  • Posts: 551
  • <AMD>< 7-32>
Re: Is this a leak?
« Reply #7 on: January 15, 2017, 03:36:38 AM »
So JJ, in this case there is not true "leak" after exit, but a dangerous "fattening" while running?

BlameTroi

  • Regular Member
  • *
  • Posts: 11
  • Blame me, everyone does
Re: Is this a leak?
« Reply #8 on: January 15, 2017, 03:41:33 AM »
So JJ, in this case there is not true "leak" after exit, but a dangerous "fattening" while running?

This is the thing I wish to avoid. I hate programs that grow during apparently normal operation and require me to restart to clear up memory. I know that nothing I expect to write will run long enough or be large enough to exhaust memory on any system I will use, but good habits are important.

Thanks all for the additional reference links. I'm now trying to settle on which debugger I want to use and learning the command sets.

 ... grumble mumble back in my day we didn't have source debuggers mumble grumble ...
"God made the integers, all else is the work of man." -- Leopold Kronecker.

anunitu

  • Member
  • ****
  • Posts: 918
Re: Is this a leak?
« Reply #9 on: January 15, 2017, 04:37:50 AM »
There was a "shareware" game from a while back(Me trying,not my program) that at about 15 minutes in (a demo of the game) started throwing memory low warnings,and just locked up at about 17-18 minutes in. The game was good with graphics and plot BUT those memory issues really shot down the idea of buying that game. Can't remember the name of the game.

jj2007

  • Member
  • *****
  • Posts: 7730
  • Assembler is fun ;-)
    • MasmBasic
Re: Is this a leak?
« Reply #10 on: January 15, 2017, 05:37:14 AM »
So JJ, in this case there is not true "leak" after exit, but a dangerous "fattening" while running?

Only if you do really stupid things, like creating a font every time in the WM_PAINT handler 8)

Quote
a) you can keep the font for the lifetime of the application (i.e. create once in WM_CREATE, use many times in WM_PAINT)
b) it makes absolutely no sense to use DeleteObject shortly before ExitProcess.

Launch Task Manager, select the gdi and user object columns. Launch the attached exe, select its row in Task Manager, then try sizing it like crazy; or move it all the time out of the screen to force repainting. There is no point in creating and deleting the same objects over and over again in a WM_PAINT handler - do it once in WM_CREATE.

sinsi

  • Member
  • *****
  • Posts: 1003
Re: Is this a leak?
« Reply #11 on: January 15, 2017, 06:11:00 AM »
Only if you do really stupid things, like creating a font every time in the WM_PAINT handler 8)
A bit like the example on MSDN.
Three CreateFont calls but only one DeleteObject ::)
I can walk on water but stagger on beer.

hutch--

  • Administrator
  • Member
  • ******
  • Posts: 4922
  • Mnemonic Driven API Grinder
    • The MASM32 SDK
Re: Is this a leak?
« Reply #12 on: January 15, 2017, 09:31:29 AM »
Troi,

Memory allocation, either yours of internally in a called function behaves like the law of gravity, what goes up must come down and one of the best habits to learn is to design your deallocation as you allocate memory. With the older GDI function which hide a massive amount of complex internal code, you need to get the swing of what internally allocates resources or memory. Things like "Device Contexts" are a left over from very early Windows version that had far less available memory and where a memory leak would eventually crash the OS. You really could write code back then that was part of a dynamic loop that would take the whole OS down but it did teach you good habits on memory and resource usage.

The trick is to design memory and resource usage so that what you allocate is tested in its de-allocation and you do this by testing the return value of the de-allocation function to make sure it worked. The normal things like keeping the allocate / de-allocate in the same scope (local in a procedure or a global if its used at different locations in an exe) ensures you don't lose the handle of the resource you have to unload.

It has always made me laugh when I see programmers slap a pile of rubbish together with loose memory allocations all over the place then try and find the leaks later with some leak detecting gizmo that often don't work. Its a case of self inflicted punishment.  :P
hutch at movsd dot com
http://www.masm32.com    :biggrin:  :biggrin:

HSE

  • Member
  • ****
  • Posts: 551
  • <AMD>< 7-32>
Re: Is this a leak?
« Reply #13 on: January 15, 2017, 09:47:16 AM »
Three CreateFont calls but only one DeleteObject ::)

Fantastic example of what Hutch say. Isn't possible delete first and second font because pointers are lost.