News:

Masm32 SDK description, downloads and other helpful links
Message to All Guests
NB: Posting URL's See here: Posted URL Change

Main Menu

GDI leak, fixed

Started by NoCforMe, September 19, 2023, 07:16:35 AM

Previous topic - Next topic

NoCforMe

I learned something today.

Had a really annoying problem: a utility program of mine that I've used since forever froze up. WTF? It's a little color-picker tool, and all the controls were non-responsive, though I could close the application.

I had the presence of mind to check it with Windows Task Manager, and sure enough, I could see the GDI object count go up dramatically every time I moved one of my sliders. It maxed out at 10,000 GDI objects: (I'd read somewhere that that was the per-process limit of GDI objects, and sure enough it is; not 10K (10,240) but exactly 10,000.) So what was going on?

After reacquainting myself with my own code of a couple years ago, I found that there was only one place where I was doing anything with GDI objects, a very short WM_PAINT handler:

;====================================================================
; Color field window proc
;====================================================================

ColorFieldProc    PROC hWin:DWORD, uMsg:DWORD, wParam:DWORD, lParam:DWORD
    LOCAL    hDC:HDC, brush:HBRUSH, oldBrush:HBRUSH, ps:PAINTSTRUCT, gpRect:RECT

    CMP    uMsg, WM_PAINT
    JE    do_paint

dodefault:
    INVOKE    DefWindowProc, hWin, uMsg, wParam, lParam
    RET

do_paint:
; Changed to use the stock DC_BRUSH instead:
;    INVOKE    CreateSolidBrush, CurrentColor
;    MOV    brush, EAX

; Get size of this window:
; No need for this--use the rect in the PAINTSTRUCT instead.
;    INVOKE    GetClientRect, hWin, ADDR gpRect

; Paint it!
    INVOKE    BeginPaint, hWin, ADDR ps
    MOV    hDC, EAX

    INVOKE    GetStockObject, DC_BRUSH
    MOV    brush, EAX
    INVOKE    SetDCBrushColor, hDC, CurrentColor
;    INVOKE    SelectObject, hDC, EAX
;    MOV    oldBrush, EAX
    INVOKE    FillRect, hDC, ADDR ps.rcPaint, brush
;    INVOKE    SelectObject, hDC, oldBrush
    INVOKE    EndPaint, hDC, ADDR ps
;    INVOKE    DeleteObject, brush        ;No need to delete stock object.
    XOR    EAX, EAX
    RET


ColorFieldProc    ENDP

(you can see all the stuff I commented out that wasn't needed)

Nowhere else in my code do I even think about device contexts or anything like that. I thought at first that it might have something to do with the controls in the program: 3 trackbars and 3 edit controls with up-down controls attached. So I disabled the paint handler: nope, now there was no leak. So it was definitely in the paint handler.

But how the hell was that leaking? I was creating a brush, then deleting it before exiting. I even saved and restored the original brush. But none of this made a difference; even if I enabled the paint handler but had it do nothing, there was still a leak.

Well, to make a long story short, I then thought, hmm, wonder if it has anything to do with the class structure (WNDCLASSEX), since the control being painted is a custom control? I took out the background brush (made it NULL): nope, no change.

Somewhat in desperation (try anything!), I added CS_CLASSDC to the class structure's style field. Voila! That fixed it. I'm not even sure why ...

WC    WNDCLASSEX <    SIZEOF WNDCLASSEX, \
            CS_HREDRAW or CS_VREDRAW or CS_BYTEALIGNWINDOW or CS_CLASSDC, \
            NULL, \        ;lpfnWndProc
            NULL, \        ;cbClsExtra
            NULL, \        ;cbWndExtra
            NULL, \        ;hInstance
            NULL, \        ;hIcon
            NULL, \        ;hCursor
            NULL, \        ;hbrBackground
            NULL, \        ;lpszMenuName
            NULL, \        ;lpszClassName
            NULL >        ;hIconSm

This structure is one of the many "set and forget" things that I've been copying and pasting for years without even thinking about it.

Interesting, because where before I was creating a brush every time the color changed (and then deleting it), I changed it to using a DC_BRUSH obtained from GetStockObject(), which I then changed the color of using SetDCBrushColor(). This is nice because you don't have to bother deleting the brush. I also stopped selecting the brush into the device context, which was unnecessary since I'm using FillRect() which uses the brush directly.

So maybe this'll help someone who has a mysterious leak they can't track down.

Oh, I attached the program here in case someone's interested. I find it useful. (Whoops, had to re-attach it; wouldn't you know it, it had a bug. Incorrect display of color hex value.)
Assembly language programming should be fun. That's why I do it.

jj2007

Quote from: NoCforMe on September 19, 2023, 07:16:35 AMSomewhat in desperation (try anything!), I added CS_CLASSDC to the class structure's style field. Voila! That fixed it. I'm not even sure why ...

Raymond Chen:

What does the CS_OWNDC class style do?

QuoteIf you register a window class and include the CS_OWNDC flag in the class styles, then the window manager creates a DC for the window and puts it into the DC cache with a special tag that means "Do not purge this DC from the DC cache because it's the CS_OWNDC for this window." If you call BeginPaint or GetDC to get a DC for a CS_OWNDC window, then that DC will always be found and returned (since it was marked as "never purge"). The consequences of this are good, bad, and worse.

What does the CS_CLASSDC class style do?

QuoteThe CS_CLASSDC class style is the same thing, but worse, for it takes all the problems of CS_OWNDC and magnifies them.

Have fun :biggrin:

.DATA
wcx WNDCLASSEXW <WNDCLASSEXW, 0, WndProc, 0, 0, 1, 2, 3, 0, 0, txClass, 4>
.CODE
WinMain proc
  wc equ [ebx.WNDCLASSEXW] ; we use an equate for better readability
  mov ebx, offset wcx
  mov wc.hbrBackground, rv(CreateSolidBrush, MyColour)
  mov wc.hInstance, rv(GetModuleHandle, 0)
  mov wc.hIcon, rv(LoadIcon, eax, IDI_APPLICATION)
  mov wc.hIconSm, eax
  mov wc.hCursor, rv(LoadCursor, NULL, IDC_ARROW)
  m2m wc.lpszMenuName, IDC_MENU
  invoke CreateWindowExW, 0, wc.lpszClassName, 0,
    guiStyle,
    _guiX, _guiY, _guiW, _guiH, ; window position: x, y, width, height
    NULL, NULL, wc.hInstance, NULL

NoCforMe

#2
Yes, I now remember reading both of those Old New Thing articles; but when I added CS_CLASSDC just now, I didn't recall them.

So it's a bad idea, eh? Wellll, first of all, I'm not at all concerned about other windows of this class being affected; this is pretty much a one-instance program. And that seemed to be Chen's chief objection to using it.

I am, however, now mystified as to what was causing all those leaks in the first place, and why using this style magically cured that. (It really did.) Any clues? I posted the only code in that program that does anything with a DC or (directly) with GDI objects.
Assembly language programming should be fun. That's why I do it.