Application crashes when generating leak reports

[TByte]

So, I really needed the leak tester to work in D7, so I've ended up removing the GetWindowTextW call from the DLL. It appears to be the only one that causes a message to be sent to my poor paralyzed application From the API docs: "If the target window is owned by the current process, GetWindowText causes a WM_GETTEXT message to be sent to the specified window or control. If the target window is owned by another process and has a caption, GetWindowText retrieves the window caption text."

The class name is sufficient (more useful in fact), to me, than window title.

For what it's worth, the diffs are:

Code: Select all

Left file: C:\Delphi\madExcept\madExcept\Dlls\madExcept32.dll (Backup Before Patch)
Right file: C:\Delphi\madExcept\madExcept\Dlls\madExcept32.dll

000960F0          56 E8 96 9F F7 FF 84 DB 0F 84 C2 00 00 00    E8
         000960F0 56 E8 96 9F F7 FF 84 DB E9 C3    00 00 00 90 E8
-----------------------------------------------------------------
000B91CF          BA 04 00 00 00 E8 13 F6 F4 FF 68 04 01 00 00 8D
000B91DF          85 CE FC FF FF 50 8B 46 20 50 E8 2E 78 F5 FF 85
000B91EF          C0 7E 36 FF 75 F0 68 F0 B7 1B 67 8D 85 94 F8 FF
         000B91CF BA 04 00 00 00 E8 13 F6 F4 FF 90 90 90 90 90 90
         000B91DF 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90
         000B91EF 90 EB 36 FF 75 F0 68 F0 B7 1B 67 8D 85 94 F8 FF
-----------------------------------------------------------------

Heck knows how that's not happening in D12, although I've had a couple or so crashes in it when generating leak report, it might be something else.

One minor thing I've noticed (sorry!!) if I click on the leak report Window while it's still visible (I moved it and left it focused initially), it crashes. This one time it crashed just when I clicked on it (different area, something circa PostMessage calls; only happened once). This is on D7, not tried D12. FWIW, the first one happens consistently here on the DestroyWindow call:

mex-DestroyWindow-crash.png (14.65 KiB) Viewed 473 times

[madshi]

Cool. I guess if it's just the one call that is problematic, I can remove it in the next build. But I suspect there will be many more, that may just not be active in your case due to your project not having leaks of that type.

[TByte]

I suppose for Window handles (if I understood the documentation of the GetWindowText call correctly), the text can be retrieved from outside which wouldn't incur a message, but talk about tons of work for miniscule benefit - just food for thought for out-of-process reporting.

Difficult to say what else might incur some execution within the EXE as a result of some APIs without seeing the full list of calls madExcept performs against leaked resources to interrogate them for information to show on the report, but from what I gathered, messages can only be sent to an HWND so this is likely the only place (but thinking about it - maybe there's some socket that's associated with a window, and maybe some info-retrieving call on that socket can cause a message to be fired off, maybe some scenario like that might popup in the future). But to save time in the future, in case anything like this happens again, madExcept could simply catch the exception and indicate to the user in some way. It's not ideal to show that leak report process crashed, but it's not ideal that it just quietly goes down. At the least, it could just abort the leak report generation and show what it's got so far, and some error message indicating there was some issue, or it could just skip that particular resource and move on to the next one (but try/excepting over AV's isn't ideal, so stopping may be the best bet). It might also help identify spurious crashes that I've been getting when clicking around the little 'creating leak report' window. It's up to you to assess the severity of these things obviously, and decide when to fix etc, but I am sure glad you're thinking of fixing this one especially as I've just reproduced it in Delphi 12 as well, but it's more rare (something to do with memory addresses still pointing to valid locations after objects are freed, so it's getting away with it more often).


This particular issue is mostly due to how Delphi designed the TPopupList and TPopupMenu implementation. If I understood correctly, instead of setting up a WndProc for each TPopupMenu, Delphi creates only 1 window and WndProc for all of them - the one created by TPopupList. With some assembly contraption I barely grasp, Delphi manages to set up this shared WndProc in such a way that it's able to retrieve the object instance of TPopupMenu and call its WndProc method (virtual - so needs to interrogate the instance a little - as far as I understand - in order to call it, rather than just doing a CALL). But on finalization, when TPopupList is freed, it does not destroy its FWindow HWND. FWindow is only destroyed when the last TPopupMenu has been freed. So, because of those unfreed TPopupMenus, the window stays up. If a message is sent to it, in the assembly in WndProc, when object instance is being retrieved, it would get a freed TPopupList.


As far as I gather, this is the only place in Delphi VCL that does this - other usages of AllocateHwnd appear to be 1 handle (AllocateHwnd) per object (e.g. TTimer).


As to why D7 is falling over every time and D12 does not isn't 100% clear, but it seems things have been rejigged so much that D12 gets away with it. The related code looks the same. In D7, the following code attempts to call [ecx+$10] while ECX is 1:

D7-Freed-TPopupList-WndProc-Call.png (41.94 KiB) Viewed 459 times

In D12, ECX points to a valid memory location most of the time for some reason, but I did have it crash on me once today on the same line (I am sure it did it before, but I can now confirm it definitely happens). In any case - D7 or D12 - entering the call gets us into procedure TPopupList.MainWndProc where Self has been freed. It's the pointers in the Freed PopupList that determine whether AV occurs or not. Looking at the TPopupList after it's been freed (when WM_GETTEXT is received):

D7-vs-D12-Freed-TPopupList-Data.png (8.79 KiB) Viewed 459 times

In D12, FList is nil, in D7 it's garbage. Something else is probably nil in D12 that makes that WndProc virtual call work. But really, I don't fully understand this - how D12 manages to somehow call it OK most of the time, against a freed object. I tried turning off ASLR (which I'm not sure if applies to 32-bit anyway) as well as turning off manifest file - no difference. On some tests, D7 got away with it also, but most of the time it crashes (e.g. if I leak TPopupMenu in FormCreate instead of in ButtonClick, for a while, it didn't crash, now it does again!). It's a fluke I think, that D12 experiences this less.

Well, I've had enough of this, as I'm sure you have Good weekend.

[madshi]

I've checked my leak reporting code and there's not a single call to PostMessage or SendMessage in it. I do call GetWindowText(), though, which apparently calls SendMessage somewhere in the depths of the OS. I've now commented that call to GetWindowText() out.

[iconic]

I do call GetWindowText(), though, which apparently calls SendMessage

Sure does It's a super thin wrapper around SendMessage(hWnd, WM_GETTEXT...);
You can call InternalGetWindowText() instead (if you need it), there is no message passing at all.

Instead, a direct syscall is made to the Win32k driver (USER32/GDI32 handling in the Shadow SDT) and it copies the updated window text directly from the kernel buffer that stores it, no SendMessage() call is performed at all.

Code: Select all

function InternalGetWindowText(hWnd: HWND; pString: PWChar; cchMaxCount: Integer): Integer; stdcall; external user32;

It's been around since Windows 2000 and persists through Windows 11.

Source: https://learn.microsoft.com/en-us/windo ... windowtext

--Iconic

[madshi]

Oh cool - thanks iconic!