5

[Updated, see bottom!]

There is a memory leak in our WinForms application hosting a WPF FlowDocumentReader in an ElementHost. I have recreated this issue in a simple project and added the code below.

What the application does

When I press button1:

  • A UserControl1 which just contains a FlowDocumentReader is created and set to be the ElementHost's Child
  • A FlowDocument is created from a text file (it just contains a FlowDocument with a StackPanel with a few thousand lines of <TextBox/>)
  • The FlowDocumentReader's Document property is set to this FlowDocument

At this point, the page renders the FlowDocument correctly. A lot of memory is used, as expected.

The problem

  • If button1 is clicked again, memory usage increases, and keeps increasing every time the process repeats! The GC isn't collecting despite loads of new memory being used! There are no references which shouldn't be there, because:

  • If I press button2 which sets elementHost1.Child to null and invokes the GC (see the code below), another weird thing happens - it will not clean up the memory, but if I keep clicking it for a few seconds, it will eventually free it!

It is unacceptable for us that all this memory stays used. Also, removing the ElementHost from the Controls collection, Disposing it, setting the reference to null, and then invoking the GC does not free up the memory.

What I want

  • if button1 is clicked mutiple times, memory usage shouldn't keep going up
  • I should be able to free all the memory (this is just one window in the "real" application, and I want to do this when it is closed)

This is not something where the memory usage doesn't matter and I can just let the GC collect it whenever. It actually ends up noticeably slowing down the machine.

The code

If you would rather just download the VS project, I've uploaded it here: http://speedy.sh/8T5P2/WindowsFormsApplication7.zip

Otherwise, here is the relevant code. Just add 2 buttons to the form in the designer and hook them up to the events. Form1.cs:

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Data;
using System.Drawing;
using System.Linq;
using System.Text;
using System.Windows.Forms;
using System.Windows.Documents;
using System.IO;
using System.Xml;
using System.Windows.Markup;
using System.Windows.Forms.Integration;


namespace WindowsFormsApplication7
{
    public partial class Form1 : Form
    {
        private ElementHost elementHost;

        public Form1()
        {
            InitializeComponent();
        }

        private void button1_Click(object sender, EventArgs e)
        {
            string rawXamlText = File.ReadAllText("in.txt");
            using (var flowDocumentStringReader = new StringReader(rawXamlText))
            using (var flowDocumentTextReader = new XmlTextReader(flowDocumentStringReader))
            {
                if (elementHost != null)
                {
                    Controls.Remove(elementHost);
                    elementHost.Child = null;
                    elementHost.Dispose();
                }

                var uc1 = new UserControl1();
                object document = XamlReader.Load(flowDocumentTextReader);
                var fd = document as FlowDocument;
                uc1.docReader.Document = fd;

                elementHost = new ElementHost();
                elementHost.Dock = DockStyle.Fill;
                elementHost.Anchor = AnchorStyles.Top | AnchorStyles.Bottom | AnchorStyles.Left | AnchorStyles.Right;
                Controls.Add(elementHost);
                elementHost.Child = uc1;
            }
        }

        private void button2_Click(object sender, EventArgs e)
        {
            if (elementHost != null)
                elementHost.Child = null;

            GC.Collect();
            GC.WaitForPendingFinalizers();
            GC.Collect();
        }
    }
}

UserControl1.xaml

<UserControl x:Class="WindowsFormsApplication7.UserControl1"
             xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
             xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
             xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006" 
             xmlns:d="http://schemas.microsoft.com/expression/blend/2008" 
             mc:Ignorable="d" 
             d:DesignHeight="300" d:DesignWidth="300">
    <FlowDocumentReader x:Name="docReader"></FlowDocumentReader>
</UserControl>

Edit:

I finally have time to deal with this again. What I tried is instead of reusing the ElementHost, disposing and recreating it each time the button is pressed. While this does help a bit, in the sense that the memory is going up and down when you spam click button1 instead of just going up, it still doesn't solve the problem - the memory is going up overall and it is not freed when the form is closed. So now I am putting a bounty up.

As there seems to have been some confusion about what is wrong here, here are the exact steps to reproduce the leak:

1) open task manager

2) click the "START" button to open the form

3) spam a dozen or two clicks on the "GO" button and watch the memory usage - now you should notice the leak

4a) close the form - the memory won't be released.

or

4b) spam the "CLEAN" button a few times, the memory will be released, indicating that this is not a reference leak, it is a GC/finalization problem

What I need to do is prevent the leak at step 3) and free the memory at step 4a). The "CLEAN" button isn't there in the actual application, it is just here to show that there are no hidden references.

I used the CLR profiler to check the memory profile after hitting the "GO" button a few times (memory usage was ~350 MB at this point). It turns out, there were 16125 (5x the amount in the document) Controls.TextBox and 16125 Controls.TextBoxView both rooted in 16125 Documents.TextEditor objects that are rooted in the finalization queue - see here:

https://i.stack.imgur.com/9RBmV.pngblah

Any insight appreciated.

Another update - Solved (kind of)

I just ran into this again in a different, pure WPF application which does not use an ElementHost or a FlowDocument, so in retrospect, the title is misleading. As explained by Anton Tykhyy, this is simply a bug with the WPF TextBox itself, it does not correctly dispose of its TextEditor.

I did not like the workarounds Anton suggested, but his explanation of the bug was useful for my rather ugly, but short solution.

When I am about to destroy an instance of a control that contains TextBoxes, I do this (in the code-behind of the control):

        var textBoxes = FindVisualChildren<TextBox>(this).ToList();
        foreach (var textBox in textBoxes)
        {
            var type = textBox.GetType();
            object textEditor = textBox.GetType().GetProperty("TextEditor", BindingFlags.NonPublic | BindingFlags.Instance).GetValue(textBox, null);
            var onDetach = textEditor.GetType().GetMethod("OnDetach", BindingFlags.NonPublic | BindingFlags.Instance);
            onDetach.Invoke(textEditor, null);
        }

Where FindVisualChildren is:

    public static IEnumerable<T> FindVisualChildren<T>(DependencyObject depObj) where T : DependencyObject
    {
        if (depObj != null)
        {
            for (int i = 0; i < VisualTreeHelper.GetChildrenCount(depObj); i++)
            {
                DependencyObject child = VisualTreeHelper.GetChild(depObj, i);
                if (child != null && child is T)
                {
                    yield return (T)child;
                }

                foreach (T childOfChild in FindVisualChildren<T>(child))
                {
                    yield return childOfChild;
                }
            }
        }
    }

Basically, I do what the TextBox should be doing. In the end I also call GC.Collect() (not strictly necessary but helps free the memory faster). This is a very ugly solution but it seems to solve the problem. No more TextEditors stuck in the finalization queue.

svinja
  • 5,495
  • 5
  • 25
  • 43
  • `StringReader` and `XmlTextReader` both have Close() methods, i´d say it can´t hurt to call them, even in a using block. Other than that i can reproduce your problem, have you tried using a Memory Profiler like [Ants](http://www.red-gate.com/products/dotnet-development/ants-memory-profiler/) ? I found that very helpful, but don´t have it installed on the machine im on right now. – Jobo Feb 07 '13 at 18:18
  • Where do you dispose your UserControl that you assign to variable uc1? Setting the child to null of your elementhost is not enough – Jehof Feb 07 '13 at 18:38
  • @Jobo I forgot to do that and am not near a computer at the moment, however I did use the .net profiler in our real application at one point. It showed a bunch of WPF objects, all eventually rooted in a System.Documents.TextEditor object (or more of them, not sure any more) in the finalizer queue. I am pretty sure the "using" is fine with the readers. – svinja Feb 07 '13 at 18:39
  • @Jehof, uc1 is a local variable so it goes out of scope, and when I set the remaining reference, elementHost1.Child, to null, it is a dead object that can be garbage collected. UserControl doesn't implement IDisposable. In fact, as I wrote in the question, the memory IS garbage collected if I just keep pressing button2 enough. – svinja Feb 07 '13 at 18:41
  • Okay i see. Its a wpf usercontrol. I tought it's winforms cause of the Form class – Jehof Feb 07 '13 at 18:52
  • can you post ur input file. – Liju Feb 23 '13 at 03:20

2 Answers2

2

I found this blog post here: Memory Leak while using ElementHost when using a WPF User control inside a Windows Forms project

So, try this in your Button2 click event:

if (elementHost1 != null)
{
    elementHost1.Child = null;
    elementHost1.Dispose();
    elementHost1.Parent = null;
    elementHost1 = null;
}

I found that calling GC.Collect() after this it may not reduce memory usage instantly, but it doesn´t increase behind a certain point. For better reproducing i made a second form, which opens your Form1. With this i tried opening your form about 20 times, always clicking on Button1 and then Button2 then closing the form, memory usage stayed constant.

Edit: The strange thing is, the memory seems to be released after opening the form again, not on GC.Collect(). I can´t help but finding this a bug with the ElementHost control.

Edit2, my Form1:

public partial class Form1 : Form
{
    public Form1()
    {
        InitializeComponent();

        m_uc1 = new UserControl1();
        elementHost1.Child = m_uc1;
    }

    private UserControl1 m_uc1;

    private void button1_Click(object sender, EventArgs e)
    {
        string rawXamlText = File.ReadAllText(@"in.txt");
        var flowDocumentStringReader = new StringReader(rawXamlText);            
        var flowDocumentTextReader = new XmlTextReader(flowDocumentStringReader);           
        object document = XamlReader.Load(flowDocumentTextReader);
        var fd = document as FlowDocument;

        m_uc1.docReader.Document = fd;

        flowDocumentTextReader.Close();
        flowDocumentStringReader.Close();
        flowDocumentStringReader.Dispose();

    }        

    private void Form1_FormClosing(object sender, FormClosingEventArgs e)
    {
        if (elementHost1 != null)
        {
            elementHost1.Child = null;
            elementHost1.Dispose();
            elementHost1.Parent = null;
            elementHost1 = null;
        }
    }

Even without explicit GC.Collect() i don´t experience any memory leak anymore. Remember, i tried this opening this form multiple times from another form.

Jobo
  • 1,084
  • 7
  • 14
  • The leak in that blog post comes from ElementHost staying referenced in the Controls collection of the parent object, while he keeps allocating new ElementHosts. On the other hand, I only have 1 ElementHost, and I keep reusing it. And as I mentioned, removing it and disposing doesn't help. – svinja Feb 07 '13 at 19:10
  • It seems to resolve the issue anyway, at least for me it did. Have you tried it? – Jobo Feb 07 '13 at 19:12
  • How does it resolve the issue for you? If you click button1 20 times in a row, the memory doesn't go up? I can't put the GC stuff everywhere or on a timer, it is extremely slow as the actual application is huge. I can afford to put it somewhere where it will run 1 time and it needs to release all the memory. Ideally, I wouldn't use it at all. And yes, I mentioned this solution not working in the question. – svinja Feb 07 '13 at 19:16
  • See my edit. Memory goes up a bit of course, but stays low and constant. – Jobo Feb 07 '13 at 19:23
  • Yes the memory will eventually go back down if I keep closing and reopening the form. This isn't because of the code in Form1_FormClosing, if you delete that code it will still happen, because a form disposes it's child controls when it is disposed. But this doesn't solve my problem, read the "what I want" part. – svinja Feb 18 '13 at 12:01
1

Indeed, PresentationFramework.dll!System.Windows.Documents.TextEditor has a finalizer and so it gets stuck on the finalizer queue (together with all the stuff hanging off it) unless disposed of properly. I scrounged around in PresentationFramework.dll a bit, and unfortunately I don't see how to get the TextBoxes to dispose of their attached TextEditors. The only relevant call to TextBox.OnDetach is in TextBoxBase.InitializeTextContainer(). There you can see that once a TextBox creates a TextEditor, it only disposes of it in exchange for creating a new one. Two other conditions under which the TextEditor disposes itself is when the application domain unloads or the WPF dispatcher shuts down. The former looks slightly more hopeful, as I found no way to restart a shut-down WPF dispatcher. WPF objects cannot be directly shared across application domains because they do not derive from MarshalByRefObject, but Windows Forms controls do. Try putting your ElementHost in a separate application domain and tear it down when clearing your form (you may need to shut down the dispatcher first). Another approach is to use MAF add-ins to put your WPF control into a different application domain; see this SO question.

Community
  • 1
  • 1
Anton Tykhyy
  • 19,370
  • 5
  • 54
  • 56
  • I think you're right, but I'm not sure how to fix it. How can I host an ElementHost in a different AppDomain? I also need data binding... I tried hosting the whole Form in another AppDomain, and shutting down the dispatcher+unloading domain+forcing garbage collection did work, but this is not viable in the actual app. What I don't understand is, if Collect/WaitForPendingFinalizers/Collect works after the AppDomain is unloaded, why doesn't it work without that and objects remain in the Finalizer queue? – svinja Feb 19 '13 at 09:44
  • Wrap the WPF-related stuff into a MarshalByRefObject-derived class, create the ElementHost together with the WPF stuffing there. Then you can CreateInstanceAndUnwrap this class and get the ElementHost, CLR will marshal it for you. If you have non-trivial data binding things become complicated and depend a lot on the exact details. After unloading the appdomain, you don't need to force a GC because unloading it removes the TextEditor objects from the finalizer queue. – Anton Tykhyy Feb 19 '13 at 11:32
  • I gave up on this. I don't like the AppDomain solution and I can't find the reason the finalizers aren't being run... Oh well. – svinja Feb 23 '13 at 11:43
  • Do you mean you have the same number of `TextEditor`s rooted in the finalizer queue before and after `GC.WaitForPendingFinalizers()`? Have you checked with WinDbg? If so, it's likely a CLR bug, report it to MS Connect if you can reproduce it in a standalone demo project. – Anton Tykhyy Feb 24 '13 at 01:28
  • Yep it doesn't change - sometimes. Sometimes it will. So if I keep clicking the button that calls collect/waitfor../collect it will eventually work. But the TextEditor objects are in the finalization queue for the entire time - at least I think they are - it looks like in the screenshot I posted. I haven't checked it with WinDbg as I've never used it. – svinja Feb 25 '13 at 10:02