6

I have a CF application that over time leaks UserControls. It took some time, but I narrowed it down, and even replicated the behavior in the full framework (3.5). Since the behavior exists in both, I don't want to call it a bug, but I sure don't understand why it's happening and hope someone can shed some light on it.

So I create a simple WinForms app with a Form and a Button. Clicking on the Button alternates between creating a new UserControl and Disposing that control. Very simple.

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

    UserControl1 m_ctl;

    private void button1_Click(object sender, EventArgs e)
    {
        if (m_ctl == null)
        {
            m_ctl = new UserControl1();
            m_ctl.Visible = true;
            this.Controls.Add(m_ctl);
        }
        else
        {
            this.Controls.Remove(m_ctl);
            m_ctl.Dispose();
            m_ctl = null;
            GC.Collect();
        }
    }
}

And here's the UserControl. It simply tracks the number of live (i.e. not finalized) instances. It has nothing on it but a single label so I can visually confirm it's on the Form.

public partial class UserControl1 : UserControl
{
    private static int m_instanceCount = 0;

    public UserControl1()
    {
        var c = Interlocked.Increment(ref m_instanceCount);
        Debug.WriteLine("Instances: " + c.ToString());

        InitializeComponent();
    }

    ~UserControl1()
    {
        var c = Interlocked.Decrement(ref m_instanceCount);
        Debug.WriteLine("Instances: " + c.ToString());
    }
}

The strange thing here is that the instance count grows indefinitely. Eventually, on the device, I run out of memory. I suspect I would on the PC as well, I'm just not inclined to click the button for the next year.

Now if I alter the UserControl's default, designer-generated Dispose method like this, simply adding the ReRegisterForFinalize call:

protected override void Dispose(bool disposing)
{
    if (disposing && (components != null))
    {
        components.Dispose();
    }

    base.Dispose(disposing);

    if (disposing)
    {
        GC.ReRegisterForFinalize(this);
    }
}

Then it behaves exactly as expected, Finalizing instances during collection (when manual or automatic).

So why is this happening? Evidently the base is calling SuppressFinalize, but exactly why would this be happening, and why in the name of Odin is it the default behavior?

ctacke
  • 66,480
  • 18
  • 94
  • 155
  • +1 for actually having a real reason to consider using GC, (sorry can't be of help on the actual solution!) – Sayse Jun 26 '13 at 22:59
  • Considering Reed's answer, what does the finalizer in your real code do? Could not calling the finalizer cause the leak? – svick Jun 26 '13 at 23:15
  • The real code has no finalizers at all. They do have Dispose implemented, and Bitmaps and the like are being disposed. – ctacke Jun 26 '13 at 23:44
  • Your repro code doesn't prove anything. You haven't eliminated the possibility of a simple bug in the Dispose() method, forgetting to dispose something. That can get you OOM when bitmaps are involved and the GC isn't running often enough. – Hans Passant Jun 27 '13 at 01:19
  • The GC runs like clockwork at a very repeatable 30s tick when the app is idle - faster when under I/O load. I'm fine with accepting that I might be off base here - it wouldn't be the first time - but what confounds me is that putting in ReRegisterForFinalize fixes the OOM (or at least delays it significantly from a system without it - I need a few days under test to be sure). – ctacke Jun 27 '13 at 01:51
  • Unfortunately the system is fairly complex, the failure takes a while to occur, and I'm running under the CF where it's a wasteland of zero profiling and debugging tools, so I'm trying to find the needle in the hidden haystack. – ctacke Jun 27 '13 at 01:54
  • And under the desktop framework and using a memory profiler, why do I see a warning about "Disposed but not GCed instance of the type" when I run the repro code, but that warning goes away when I put in my hack? – ctacke Jun 27 '13 at 01:58
  • @ctacke Did the memory profiler say why they weren't GCed? And which one is it, GCed or finalized? Because they aren't the same thing. – svick Jun 27 '13 at 04:27
  • I doesn't say why. It said "not GCed", so my assumption is that it's still hanging out there in memory. I'm trying to parse the output as to the "why" - it's got to have some root holding it, but there is an overload of info in this tool. – ctacke Jun 27 '13 at 14:28

1 Answers1

4

So why is this happening? Evidently the base is calling SuppressFinalize, but exactly why would this be happening, and why in the name of Odin is it the default behavior?

That is the default behavior for classes (properly) implementing IDisposable. When you call IDisposable.Dispose, the default, suggested behavior is to suppress finalization, since the main reason for finalization is to clean up resources that were never disposed. This is because finalization is an expensive operation - you don't want to finalize objects unnecessarily, and if Dispose was called, the thought is that you've already cleaned up your non-managed resources. Any managed memory will get handled regardless.

You should override Dispose, and do your decrement within the Dispose override.

This behavior is explained in the documentation for IDisposable. The sample Dispose method call implementation is (from the referenced documentation):

public void Dispose()
{
    Dispose(true);
    // This object will be cleaned up by the Dispose method. 
    // Therefore, you should call GC.SupressFinalize to 
    // take this object off the finalization queue 
    // and prevent finalization code for this object 
    // from executing a second time
    GC.SuppressFinalize(this);
}
Reed Copsey
  • 554,122
  • 78
  • 1,158
  • 1,373
  • The only problem with this is that in the actual app environment, memory crawls up as you navigate around the application, and GC never recovers that memory. Luckily all of my Views derive from a single base UserControl. When I ReRegisterForFinalize in that base, the leak goes away. None of my Views have implemented finalizers. By association, this seems to be the cause of the trouble. – ctacke Jun 26 '13 at 23:43
  • What turned me down this path was using the .NET Memory Profiler, which said there were views that were "disposed but not finalized". – ctacke Jun 26 '13 at 23:45