winforms icon indicating copy to clipboard operation
winforms copied to clipboard

Releasing leak objects of UiaTextRange

Open vladimir-krestov opened this issue 2 years ago • 2 comments

Fixes #7248

Proposed changes

  • Use HandleDestroyed events to release related objects of UiaTextRange objects. We create new instances of the ranges and send them to UIA (eg. Inspect). When TextBox disposing UIA tool keeps several ranges thereby keep the provider and the TextBox accessible object. Due to we have many unregistered text ranges we don't have access to all of them, so I think it would be better to use events and subscription on them for every text range.

Customer Impact

  • Application slows down or hangs to perform GC
  • This change related to DataGrdiViewTextBoxCell, PropertyGrid edit field, ComboBox, Numeric/DomainUpDown edit field, etc. TextPatterns leaks affect all controls with edit field and keep all their leaked objects, so this fix resolves it.

Regression?

  • No

Risk

  • We make some fields as null. UIA can call some method or property after disposing, thereby try to get some value of null-object. So we can catch NRE. To avoid it needs to add necessary null checks.

Screenshots

Before

  • UIA keeps text ranges and TextBox accessible object
  • TextBox accessible object keeps TextBox control (will be fixed next) image image

After

  • TextBox text provider is released image

Test methodology

  • Manual testing
  • CTI

Accessibility testing

  • Using Inspect and Narrator:
    • Focus on a TextBox
    • Navigate through characters
    • Select some range
    • Use Inspect TextPattern explorer to get all possible ranges

Test environment(s)

  • dotnet 7.0.0-preview.5.22272.3
  • Windows 11
Microsoft Reviewers: Open in CodeFlow

vladimir-krestov avatar Jun 13 '22 14:06 vladimir-krestov

I investigated if it's possible to use one UiaTextRange instance for all ranges, it breaks TextPattern. I investigated ways to release UiaTextRange objects, but didn't find any. Narrator keeps several ranges.

vladimir-krestov avatar Jul 08 '22 10:07 vladimir-krestov

How many text providers are created per text box?

Consider factoring out cleanup changes in textboxbase and combobox.....cs and submitting a small PR for them

Tanya-Solyanik avatar Aug 04 '22 00:08 Tanya-Solyanik

@dmitrii-drobotov - this is not the highest priority issue, but please factor out changes that test for AcccessibileObject being already created and IsHandleCreated, etc, into a smaller PR, if still applicable, and then test what object holds the TextBox, i.e. run !GCRoot on the textbox address. I think it might be OK for the UIAProvider to leak as long as the text box is released.

Tanya-Solyanik avatar May 31 '23 17:05 Tanya-Solyanik

@Tanya-Solyanik smaller changes from this PR are already implemented: https://github.com/dotnet/winforms/blob/ea36ea9229ef001ffe21629d1c4e2fb3047a04f9/src/System.Windows.Forms/src/System/Windows/Forms/TextBoxBase.cs#L1581-L1587 Instead of IsHandleCreated there are owner existence checks, but I think it's equivalent: https://github.com/dotnet/winforms/blob/ea36ea9229ef001ffe21629d1c4e2fb3047a04f9/src/System.Windows.Forms/src/System/Windows/Forms/TextBoxBase.TextBoxBaseUiaTextProvider.cs#L101-L103 https://github.com/dotnet/winforms/blob/ea36ea9229ef001ffe21629d1c4e2fb3047a04f9/src/System.Windows.Forms/src/System/Windows/Forms/TextBoxBase.TextBoxBaseUiaTextProvider.cs#L137-L140

The part with unlinking TextBox control and its AccessibleObject is also implemented in #9400, so there is no need for a follow-up work.

I additionally tested current state of UiaTextRange memory leak, and actually most of objects are collected on latest main. Result of running !dumpheap -type Text after interacting with text boxes while form is still opened:

> !dumpheap -type Text
Statistics:
              MT    Count    TotalSize Class Name
00007ff8bd6f9058        1           40 Windows.Win32.PInvoke+SetTextColorScope
00007ff8bd6f8f48        1           40 Windows.Win32.PInvoke+SetTextAlignmentScope
00007ff8bd171440        1           48 System.Text.UTF8Encoding+UTF8EncodingSealed
00007ff8bd729e58        1           56 System.Text.DecoderNLS
00007ff8bd6471a0        1           68 System.Windows.Forms.TextImageRelation[]
00007ff8bd17f730        3           72 System.Text.EncoderReplacementFallback
00007ff8bd17f4a8        3           72 System.Text.DecoderReplacementFallback
00007ff8bd7d58a8        4           96 System.WeakReference`1[[System.Windows.Forms.TextBoxBase, System.Windows.Forms]]
00007ff8bd7d5690        4           96 System.Windows.Forms.TextBoxBase+TextBoxBaseUiaTextProvider
00007ff8bd7d1d30        2           96 System.Windows.Forms.RichTextBox+OleCallback
00007ff8bd72a7c8        2           96 System.Text.UnicodeEncoding
00007ff8bd7d4ca0        4          320 System.Windows.Forms.TextBox+TextBoxAccessibleObject
00007ff8bd7a4f88        1          544 WinformsControlsTest.TextBoxes
00007ff8bd7a8b50        2          672 System.Windows.Forms.RichTextBox
00007ff8bd7d8450       36         1152 Interop+UiaCore+ITextRangeProvider[]
00007ff8bd7a70a8        4         1184 System.Windows.Forms.TextBox
00007ff8bd7d8eb8       68         1632 System.Windows.Forms.Automation.TextDecorationLineStyle
00007ff8bd7d8130      366        14640 System.Windows.Forms.Automation.UiaTextRange
Total 504 objects

After closing the form and forcing GC:

> !dumpheap -type Text
Statistics:
              MT    Count    TotalSize Class Name
00007ff8bd8260e0        1           32 System.Text.InternalEncoderBestFitFallback
00007ff8bd7d2f88        1           32 System.Text.CodePagesEncodingProvider
00007ff8bd825fa8        1           40 System.Text.InternalDecoderBestFitFallback
00007ff8bd171440        1           48 System.Text.UTF8Encoding+UTF8EncodingSealed
00007ff8bd729e58        1           56 System.Text.DecoderNLS
00007ff8bd6471a0        1           68 System.Windows.Forms.TextImageRelation[]
00007ff8bd7d58a8        3           72 System.WeakReference`1[[System.Windows.Forms.TextBoxBase, System.Windows.Forms]]
00007ff8bd7d5690        3           72 System.Windows.Forms.TextBoxBase+TextBoxBaseUiaTextProvider
00007ff8bd17f730        3           72 System.Text.EncoderReplacementFallback
00007ff8bd17f4a8        3           72 System.Text.DecoderReplacementFallback
00007ff8bd8242f0        1           80 System.Collections.Generic.Dictionary`2[[System.Int32, System.Private.CoreLib],[System.Text.Encoding, System.Private.CoreLib]]
00007ff8bd6f9058        2           80 Windows.Win32.PInvoke+SetTextColorScope
00007ff8bd6f8f48        2           80 Windows.Win32.PInvoke+SetTextAlignmentScope
00007ff8bd8266b8        1           96 System.Collections.Generic.Dictionary`2+Entry[[System.Int32, System.Private.CoreLib],[System.Text.Encoding, System.Private.CoreLib]][]
00007ff8bd72a7c8        2           96 System.Text.UnicodeEncoding
00007ff8bd8257b0        1          136 System.Text.SBCSCodePageEncoding
00007ff8bd7d4ca0        3          240 System.Windows.Forms.TextBox+TextBoxAccessibleObject
00007ff8bd7d8130        8          320 System.Windows.Forms.Automation.UiaTextRange
Total 38 objects

There are still some UiaTextRange objects left, but nearly not as much as before, and TextBox controls with TextBoxes form are garbage collected.

dmitrii-drobotov avatar Jul 12 '23 13:07 dmitrii-drobotov