New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
HwndWrapper leaks class registrations (ATOMs returned by RegisterClassEx) #9026
Comments
Please provide a repro project. I have WPF apps running for weeks and months without this issue, so it's not as simple as that, probably depends on what it is doing.
wpf/src/Microsoft.DotNet.Wpf/src/Shared/MS/Win32/HwndWrapper.cs Lines 308 to 311 in 06d23fe
|
ClassRegistrationLeak.zip |
Thanks @bbmmcodegirl! From my initial look the HwndWrapper is being diposed fine which tries to enqueue the wpf/src/Microsoft.DotNet.Wpf/src/WindowsBase/System/Windows/Threading/Dispatcher.cs Lines 2496 to 2503 in 06d23fe
It feels that all things are the way they are supposed to, but the dispatcher never gets to dispatch the DestroyWindow operation. I am suspecting this might be because you simply terminate the thread without properly telling the dispatcher. Calling There is some way to access the table, see e.g. https://github.com/JordiCorbilla/atom-table-monitor |
Hi @miloush (Jan). Thanks for confirming that something appears not to be working as it should. But we have run into this in the real world, I only made this example to show how it's happening. In the real world, we are calling Also in the real world, we have, of course, now implemented a different workaround where we simply don't call But this problem has caused us and our customers problems until we found out about this. We ran into this problem unwittingly. And so might someone else. "There is some way to access the table, see e.g. https://github.com/JordiCorbilla/atom-table-monitor". More to the point, the Task Manager has a column |
Well if you are the one who creates a dispatcher then it seems fair you should be the one to shut it down. I am not quite sure what you are hoping WPF would do here else other than offer you a way to unregister the class. Can you create your own thread with dispatcher and then shut it down when you are done? Or keep one dispatcher for yourself that you would use to invoke the code you need.
The releases of the repo has a zip file which has ATOMTableMonitor.exe. It does list the HwndWrapper registrations as RWM. |
Wow, stumbling upon this issue shocked me for some degree, because I wanted to create the very exact issue last Friday, but because of time shortage I postponed it to today and realized someone created it already. Last week we encountered the exact same problem. We had some The first time we stumbled upon that problem was when we tried to offload image loading to background threads to make the UI thread more responsive. If again the application runs for a long enough amount of time the atom table will be exhausted which then also destabilizes the whole system.
I kind of have to disagree on that. Most WPF APIs do not hint in any way that a In my opinion the fail in design comes from Some idea to tackle this problem could be to disallow the implicit creation of We should also consider to move the creation of the |
It could have been called
Note that this will not solve the reported issue, because the CommandManager posts an operation to the dispatcher and relies on the message to the hwnd wrapper to process the queue. And it will not fix the "leak" anyway if you push frame and then terminate the thread. I am not saying (yet) the situation cannot be improved, but I don't see any obvious improvement opportunities without breaking existing code. |
@czdietrich thanks for posting your problem with the same issue. @miloush "the issue - and so far it appears to me to be the reason of the leak - is the unexpected termination of the dispatcher thread." On one hand, you are right, on the other hand, in my case for example, the creator of the thread lies somewhere buried in a legacy library that was used. And this is not the fault of the (current) implementer. At the time of creation the thread is a complete background thread, it's not a dispatcher thread. It only becomes one because an unwitting developer used an innocuous-looking WPF method in it, not even on their own fully aware that ( Nobody is "shutting down the thread", the thread happens to accidentally end because at the place where it was created, in the library, the thread function ended. If it had been a Task instead of a Thread, the thread would have survived the end of the function. I agree that it would probably be hard to fix that without breaking existing code. I go with czdietrich's suggestion, at some point there could be an event raised. Maybe even an exception thrown. Imagine the signature of the method was At the very least there should be a more explicit warning about explicitly created Threads in the documentation than just the very weak statement "In .NET Framework 4, the TPL is the preferred way to write multithreaded and parallel code." in https://learn.microsoft.com/en-us/dotnet/standard/parallel-programming/task-parallel-library-tpl. |
.. a dumb question on the side. Why does the UnregisterClass have to happen on the same thread that the class was registered on? |
Still I think the decision to make
I think the following happens for us:
This is ok from my side, calling
I'm not sure about this, but maybe we point to different things here. |
I would guess that https://learn.microsoft.com/en-us/windows/win32/api/winuser/nf-winuser-destroywindow#remarks |
In general I would think it is "not polite" to create a dispatcher on thread that doesn't belong to you. I am sure there are other ways the thread owner can "screw up" the dispatcher than just exiting the thread. I understand that the issue is that people are generally not aware that something creates a dispatcher. There isn't that many It sounds like if |
I agree, but the issue is not really that they require a There are even some more things to consider here: |
@czdietrich wrote: "offload to a thread pool thread using @miloush says "it's not really a problem until the thread exits or is terminated" But in the case described by @czdietrich, the call chain was invoked with Task.Run(), and the threadpool thread would still be running. Who is right? Can we even leak resources in the same way when the parent call was made on a threadpool thread? |
To clarify, I am attributing the post message failure to the thread being gone, because the other option (of message flooding) seems unlikely at this point. I didn't see the actual error code. I would also assume that the task scheduler has freedom to manage threads as it sees fit. If it keeps it alive at the time of GC of the dispatcher, it either should work or the cause is different (but not any more helpful). If it decides to span new threads we are back at the original scenario. Either way, the fix is to not create dispatchers on threads you don't control. I believe we are in agreement here with @czdietrich and that the problem is in dispatchers being created without developers suspecting it. Unfortunately it seems the best we can do is document the cases somewhere. |
Description
The implementation of
MS.Win32.HwndWrapper
leaks Win32 Windows class registrations.MS.Win32.HwndWrapper
in its constructor calls the Win32 APIRegisterClassEx
function (and then creates a window with it usingCreateWindowEx
).It holds unmanaged resource, so it also implements the Dispose pattern, and in its
internal Dispose(bool, bool)
it will schedule the correspondingUnregisterClass
function. But it will only do that if the second parameter toDispose(bool, bool)
, calledisHwndBeingDestroyed
, is true. AndDispose(bool, bool)
is called in two locations, the ordinaryDispose()
method that implementsIDisposable
, and in the finalizer. But in both calls, the second parameter isfalse
.isHwndBeingDestroyed
is never true. The class will never be unregistered.As it happens, the ATOM returned by
RegisterClassEx
is is from the User Atom Table for which there is no useful API, so it is not possible to inspect this atom table or maybe search for old class registrations lying around. It also appears to be the case that these class registrations persist even when the process exits. This means that a WPF application that creates aHwndWrapper
anywhere slowly eats up the available Windows class registrations. And only 16,384 are available in total. If an application unwittingly manages to exhaust them, this renders the operation system unusable. No window can be created anymore, not a Task Manager to shut down the process, not a Shutdown dialog with the option to shut down or restart the machine. This is fatal.The
User Atom Table
is not really documented (or I haven't found the documentation), so it is not clear to me if it gets cleared when the user logs out, or only when the system restarts. It appears to definitely not get cleared when the process exits, judging from the effect it can have. From the name 'UserAtomTable' it sounds like it should be cleared when logging out.Reproduction Steps
Warning. If you want to reproduce this, be prepared to restart your operating system afterwards, or at least log off. Have a command window open already to do that. Best try this only in a VM.
HwndWrapper
is used in various parts of WPF. Here is a code snippet that demonstrates the effect, I have tried it:When this comes to its Win32 exception, the system's class atom's are exhausted, and you better restart the machine (or log out). At the very least, close some windows.
You may ask what the real-world scenario is here, so why one should deliberately call
Dispatcher.CurrentDispatcher
on lots of different threads.Dispatcher.CurrentDispatcher
butCommandManager.InvalidateRequerySuggested()
which internally callsDispatcher.CurrentDispatcher
.Expected behavior
Expected behavior would be to not to leak any class handles. Since class handles are invisible and the User Atom Table cannot be inspected, there is no obvious symptom at all except that the system will be unresponsive all of a sudden:
Actual behavior
The actual behavior is described above. If a WPF application manages to exhaust the available class registrations, it will get the error "Not enough memory resources are available to process this command" (Native error 8). But in addition, this makes the operation system completely unusable. No window can be created anymore. Not even the Task Manager, nor the Performance Monitor, nor the Shutdown Dialog can be created unless some other window is closed. If a user doesn't have the wits to control-tab to some other window, close it, and then start an orderly shutdown or logout, all they can do is hard-reset the system, i.e. use the power button to switch the computer off.
Regression?
From my code inspections, this is the case in .Net Framework 4.7.2 as well as the current code from dotnet core, and for sure all other versions in between.
Known Workarounds
If the depletion of windows class handles is very rapid, it will usually be because of an error. Normally it should not be necessary to use
Dispatcher.CurrentDispatcher
on lots of different threads. It should not even be necessary to create many threads (new Thread()
) in the first place. Even if someone makes a mistake and callsDispatcher.CurrentDispatcher
on a background thread, if this thread is from the threadpool (Task.Run()
), the number of lost class handles will be limited to at most the number of threadpool threads.But there is no working around that the handles stay leaked even when the process exits, and so the only workaround is to restart the system occasionally.
Impact
Here is a story of some of the impact this had.
https://stackoverflow.com/questions/78319645/long-running-process-with-wpf-front-end-on-a-windows-ipc-fails-with-not-enough
On an actual machine out there in the real world, this failed a company's production process in progress, made some of their products unusable, and harmed the reputation of the manufacturer of the machine.
Configuration
As stated above, this code seems to be the same from at least .Net Framework 4.7.2 to now.
Operating system will be Windows, and it is a matter of the Win32 API, which underpins all versions of Windows AFAIK. any architecture. "Not enough memory resources are available to process this command" is an error of the Win32 API when
RegisterClassEx
fails. The problem will be happening until either WPF starts cleaning up the class registrations correctly, or Win 32 will start cleaning them up when a process exits.Other information
https://referencesource.microsoft.com/#q=HwndWrapper
https://github.com/dotnet/wpf/blob/main/src/Microsoft.DotNet.Wpf/src/Shared/MS/Win32/HwndWrapper.cs
The text was updated successfully, but these errors were encountered: