GDI leak driving me nuts

[NoCforMe]

OK, here's one for you: a simple, simple WM_PAINT handler that's causing GDI leaks. And I can't figure out why.

Here's the offending code:

Code Select Expand

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

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

MOV EAX, uMsg
CMP uMsg, WM_NCCREATE ;Must return TRUE
JE true_exit ; for control creation to proceed.
CMP EAX, WM_PAINT
JE do_paint

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

do_paint:
INVOKE BeginPaint, hWin, ADDR ps
MOV hDC, EAX

; INVOKE GetStockObject, DC_BRUSH
; MOV brush, EAX
; INVOKE SetDCBrushColor, hDC, CurrentColor

INVOKE CreateSolidBrush, CurrentColor
MOV brush, EAX

INVOKE FillRect, hDC, ADDR ps.rcPaint, brush

INVOKE DeleteObject, brush
INVOKE EndPaint, hDC, ADDR ps
XOR EAX, EAX
RET

true_exit:
MOV EAX, TRUE
RET

ColorFieldProc ENDP

And no, I'm not going to post the entire program's code, at least not just yet, because I can tell you for sure that this is where the problem lies, though I have no idea why. If I stop processing WM_PAINT messages, the problem goes away and there is no GDI leak. (Of course then I get no display from this window.) I have included the executable here so you can see the leak for yourself; fire up the program, open Task Manager, slide the sliders around and see the number of GDI objects climb higher and higher. (It's definitely NOT the trackbars; as I said, if you disable the paint handler, there's no GDI leak.)

The program is a simple color displayer that lets you play around with RGB colors via trackbars; this proc just displays the color in a custom control window.

The commented-out code in the paint handler is the alternative I tried to creating a brush using CreateSolidBrush(); I tried GetStockObject (DC_BRUSH), then using SetDCBrushColor(hDC, CurrentColor) to change the color.

Either way makes no difference; I get the same leak.

Help!

[_japheth]

OK, here's one for you: a simple, simple WM_PAINT handler that's causing GDI leaks. And I can't figure out why.
...
Help!

Since you're sure the error is within WM_PAINT, I'd suggest to examine the return value of DeleteObject() - and, if it's NULL, call GetLastError() to see why it failed.

[NoCforMe]

OK, will try. May be grasping at straws, but who knows?
Nope, that wasn't it:

[jj2007]

I've built your code into a standard WM_PAINT handler, no leak - as expected. It's elsewhere. Without complete code, we'll never find out.

[NoCforMe]

OK, here 'tis:

[jj2007]

Ok, I can confirm that it's a weird problem. No errors, but EndPaint, according to Microsoft: The return value is always nonzero

Here, it always returns zero.

I guess it should be DefDlgProc, but the problem persists:

dodefault:
   INVOKE   DefDlgProc, hWin, uMsg, wParam, lParam

[_japheth]

I also had a look at this code ... it's a bit strange IMO, but perhaps that impression is due to my dwindling windows knowledge.

A modal dialog box is created within WinMain, and IIRC such a strategy needs a different handling than creating a simple window. There's a) no need to implement a message loop within WinMain, because Windows itself handles the message processing within the DialogBoxIndirectParam() function. And b), your dialog proc is supposed to exit by calling EndDialog(), not by calling PostQuitMessage().

[jj2007]

Minor modifications, and it works now

However, the real cause of the leak remains obscure. Personally, I hate dialog applications - their behaviour is opaque, and badly documented.

[HSE]

Hi NoC!

Apparently there is some problem processing controls messages.

Look like must be:

Code Select Expand

do_hscroll:
; Get trackbar handle (in lParam), see which one sent us this message:
MOV EAX, lParam
CMP EAX, RtrkHandle
JE do_red
CMP EAX, GtrkHandle
JE do_green
CMP EAX, BtrkHandle
JE do_blue
        xor eax, eax
        ret

do_blue:
; Handle blue trackbar:

You code have some kind of "martir style" that don't help to find what other thing can be wrong. I don't like to suffer in that way

Regards, HSE.

[Greenhorn]

A Dialog Prcedure should return TRUE if it handles a message (with a few exceptions).

https://learn.microsoft.com/en-us/windows/win32/api/winuser/nc-winuser-dlgproc

Type: INT_PTR

Typically, the dialog box procedure should return TRUE if it processed the message, and FALSE if it did not. If the dialog box procedure returns FALSE, the dialog manager performs the default dialog operation in response to the message.

[NoCforMe]

And b), your dialog proc is supposed to exit by calling EndDialog(), not by calling PostQuitMessage().

No, that's not correct, at least by empirical evidence. Perhaps it's a special case, since my dialog is the main window (i.e., I'm not invoking the dialog from another window but from my WinMain() procedure), but EndDialog() will indeed end the dialog but not the program, which remains stranded until closed by Task Manager. So PostQuitMessage() is indeed the correct thing here. It's what I've been doing in all my programs that are built as dialog-as-main-window since forever.

You code have some kind of "martir style" that don't help to find what other thing can be wrong. I don't like to suffer in that way

Intriguing thought, but what exactly do you mean here by "martir" (should be martyr) style? Is my code trying to commit suicide?

A Dialog Prcedure should return TRUE if it handles a message (with a few exceptions).

https://learn.microsoft.com/en-us/windows/win32/api/winuser/nc-winuser-dlgproc

Type: INT_PTR

Typically, the dialog box procedure should return TRUE if it processed the message, and FALSE if it did not. If the dialog box procedure returns FALSE, the dialog manager performs the default dialog operation in response to the message.

This is an interesting one, because it is apparently in conflict with the suggested return values for the messages themselves, like say WM_PAINT where Micro$oft says "An application returns zero if it processes this message". So I'm confused.

In any case, I changed things so that I return TRUE to all messages that I process; made absolutely no difference, either in program operation or in the GDI leak.

It seems that maybe JJ is onto something here, since he found that EndPaint() was returning zero instead of the nonzero that MS says it "always returns". Hmmmm ...

[HSE]

Intriguing thought, but what exactly do you mean here by "martir" (should be martyr) style?

Glad to know you are learning spanish, muy bien   

An important feature of macros and hll syntax is to improve code readability. Other way you "voluntary accept an evitable suffering" (in the name of baremetal code?).

 

[NoCforMe]

Regarding learning Spanish (I haven't learned much), I feel every American should be required to know that language. I know enough to order food in a restaurant ...

But mi amigo, regarding your critique of my code regarding "bare metal-ism" (btw, no offense taken here), I think you're too addicted to macros and "high-level" formatting, a topic on which I'll have much more to say here soon (though certainly not in this subforum since that's a highly opinionated discussion!).

Let's take that piece of code you objected to (and wrongly "corrected"):

Code Select Expand

do_hscroll:
; Get trackbar handle (in lParam), see which one sent us this message:
MOV EAX, lParam
CMP EAX, RtrkHandle
JE do_red
CMP EAX, GtrkHandle
JE do_green
CMP EAX, BtrkHandle
JE do_blue
        xor eax, eax
        ret

do_blue:
; Handle blue trackbar:
(your "correction" in lowercase at the end)

If you bothered to actually follow this code, which I admit is written in classic assembly format and doesn't use fancy "case/switch" macros, you'd see that it's looking at trackbar handles to see which one generated the WM_HSCROLL message. Since I know for a fact that there are only 3 such controls that could possibly generate such a message here, I leave the last case (the blue trackbar) to simply drop through to its handler code; no need to exhaustively check every other possibility. So no, I definitely don't want that return there. Which I would think would be obvious from looking at this simple code, but I guess not.

Anyhow, GDI leak still continues merrily on, so back to our regularly scheduled program ...

[HSE]

So no, I definitely don't want that return there

That it's the logic thing. But logic can fail if only one assumption is incorrect

Look like modification improve program behaviour, if you tested that (not thinked that) was just coincidence here.

[Greenhorn]

Sometimes one does not see the forest for the trees ... 

INVOKE    EndPaint, hWin, ADDR ps