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:
;====================================================================
; 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!
Quote from: NoCforMe on October 09, 2023, 03:46:56 PMOK, 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.
OK, will try. May be grasping at straws, but who knows?
Nope, that wasn't it:
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.
OK, here 'tis:
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
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().
Minor modifications, and it works now :thumbsup:
However, the real cause of the leak remains obscure. Personally, I hate dialog applications - their behaviour is opaque, and badly documented.
Hi NoC!
Apparently there is some problem processing controls messages.
Look like must be:
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 :biggrin:
Regards, HSE.
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
QuoteType: 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.
Quote from: _japheth on October 09, 2023, 10:37:13 PMAnd 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.
Quote from: HSE on October 10, 2023, 01:05:08 AMYou 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 :biggrin:
Intriguing thought, but what exactly do you mean here by "martir" (should be
martyr) style? Is my code trying to commit suicide?
Quote from: Greenhorn on October 10, 2023, 03:31:56 AMA 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
QuoteType: 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 ...
Quote from: NoCforMe on October 10, 2023, 05:19:05 AMIntriguing thought, but what exactly do you mean here by "martir" (should be martyr) style?
Glad to know you are learning spanish, muy bien :biggrin: :biggrin:
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?).
:thumbsup:
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"):
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 ...
Quote from: NoCforMe on October 10, 2023, 06:38:38 AMSo 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 :biggrin:
Look like modification improve program behaviour, if you tested that (not thinked that) was just coincidence here.
Sometimes one does not see the forest for the trees ... :greensml:
INVOKE EndPaint, hWin, ADDR ps
Quote from: NoCforMe on October 10, 2023, 05:19:05 AMQuote from: _japheth on October 09, 2023, 10:37:13 PMAnd 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.
Japheth's statement is correct. I have several applications which use a Dialog Box as the main window and no problems with exiting the process.
Just call EndDialog, hWin, TRUE and in WinMain return the return value of DialogBox... function.
I trtied it in your application (and removed the useless message pump) and it works properly. :thumbsup:
Quote from: Greenhorn on October 10, 2023, 07:13:34 AMSometimes one does not see the forest for the trees ... :greensml:
INVOKE EndPaint, hWin, ADDR ps
That was it. Bingo.
You, esteemed coder, win the grand debugging prize. (As well as the careful-reading prize.) The GDI object count climbs up just a little bit (47 in my trial) and stays there.
My hat is off to you!
I want to thank the congregation here for helping me and for solving this problem. I learned two things:
1. The message loop I have in all my dialog-as-main-window programs is totally unnecessary. I'll be getting rid of them.
2. Don't miss the forest for the trees.
Thanks again!
Congrats, Greenhorn - you spotted what nobody else saw :thumbsup:
I had a suspicion that it was the DC - see line 62 of the attached code above: there was no leak with CS_OWNDC (meaning Windows reused it all the time, and didn't mind that it was recreated a hundred times with BeginPaint).