Fix Issue 14721: TreeView.CustomDraw leaks one HFONT per visible item per paint when NodeFont is set, leading to GDI handle exhaustion and crash#14730
Conversation
… per paint when NodeFont is set, leading to GDI handle exhaustion and crash
There was a problem hiding this comment.
Pull request overview
This PR addresses a GDI handle leak in TreeView.CustomDraw by avoiding per-paint HFONT creation when TreeNode.NodeFont is set, reducing the risk of GDI handle exhaustion and process crashes during frequent repaint scenarios.
Changes:
- Switches the selected font handle in the custom draw path from a short-lived
OwnerDrawPropertyBaginstance to the node’s long-lived_propBagcache. - Adds guards/asserts around
_propBagusage when selecting the font into the item DC.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| using Font secondFont = new(SystemFonts.MenuFont, FontStyle.Bold); | ||
|
|
||
| bag.Font = firstFont; | ||
| _ = bag.GetType().GetProperty("FontHandle", BindingFlags.Instance | BindingFlags.NonPublic)!.GetValue(bag); |
There was a problem hiding this comment.
ontHandle is internal and directly accessible in the test assembly (InternalsVisibleTo), so the reflection lookup for it is unnecessary
| bag.Font = firstFont; | ||
| _ = bag.GetType().GetProperty("FontHandle", BindingFlags.Instance | BindingFlags.NonPublic)!.GetValue(bag); | ||
|
|
||
| FieldInfo fontWrapperField = bag.GetType().GetField("_fontWrapper", BindingFlags.Instance | BindingFlags.NonPublic)!; |
There was a problem hiding this comment.
Consider using TestAccessor.Dynamic instead of reflection.
| // Mess with the DC directly... | ||
| PInvokeCore.SelectObject(nmtvcd->nmcd.hdc, renderinfo.FontHandle); | ||
| Debug.Assert(node._propBag is not null); | ||
| if (node._propBag is not null && node._propBag.Font is not null) |
There was a problem hiding this comment.
The node._propBag.Font is not null part is redundant with the higher-level check renderinfo.Font is not null. GetItemRenderStyles sets retval.Font = node._propBag.Font (same reference) and returns an empty bag (Font == null) whenever node/node._propBag is null, so once renderinfo.Font is not null holds, node._propBag.Font is guaranteed non-null too and this inner check can be dropped. (Keep the node._propBag is not null half only to satisfy the compiler's nullable analysis, which can't infer that invariant across the method boundary.)
Fixes #14721
Root Cause
TreeView.CustomDrawcallsGetItemRenderStylesduring everyITEMPREPAINTevent, and this method creates a newOwnerDrawPropertyBag. The original code utilized renderinfo.FontHandle, which triggeredFont.ToHfont()—allocating a newHFONT—whenever the property was first accessed in a new bag. Because these short-lived objects could not be released immediately, GDI font handles accumulated rapidly during high-frequency repainting, eventually leading to handle exhaustion and a crash.Proposed changes
renderinfo.FontHandletonode._propBag.FontHandle, utilizing the reusable, long-lived cache available on theTreeNode. Null checks (fornode._propBagandnode._propBag.Font) are retained to ensure safety. This change preserves standard rendering semantics (using the same font source) while avoiding the repeated creation of anHFONTduring every paint operation.OwnerDrawPropertyBag, changeFontfrom an auto-property to one with a custom setter: when the font changes, explicitly dispose of the old_fontWrapperand set it tonull, ensuring that the correctHFONTis recreated the next timeFontHandleis accessed.Customer Impact
Win32Exception: The operation completed successfully.) and subsequent application crashes (0xC000041D) caused by reaching the 10,000 GDI handle limit.TreeNode.NodeFontat runtime, the new font is correctly reflected, and the oldHFONTis not used.Regression?
Risk
Screenshots
Before
14721-Before.mp4
After
14721-After.mp4
Change the font of a TreeNode:
14721-After2.mp4
Test methodology
Test environment(s)
Microsoft Reviewers: Open in CodeFlow