C# winforms possible memoryleak when adding and removing custom controls to flowlayoutpanel (many User objects in taskmanager)

Question

I have a medium-sized Winforms application (dotNET4.0) on which I'm dynamically adding and removing custom controls from a flowlayoutpanel.
Based on user selections, the amount of these controls vary.

This is working fine, except that I notice some memoryleaks. I check this by monitoring the 'USER Objects' number in Taskmanager. The number goes up when I'm adding the custom controls to the flowlayoutpanel but doesn't go all the way down again when disposing these.

Actually: the number of USER Objects goes down a lot (lets say from 100% to 10%: so 90% is properly disposed, leaving me with 10% in memory)...

Concluding: one or more objects still remain in memory after disposing the user control. My guess is the delegates or the images, but I'm clueless... Could it be something from the static?

So my actual question is: Where am I not properly freeing memory from the user controls and how can I resolve this? Could you guys please assist?

Many many thanks in advance!

This is my user control:
Note that everything can be disposed EXCEPT the _Costlinereportdata object.
(this should remain in use because it is linked and used in other parts)

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Drawing;
using System.Data;
using System.Linq;
using System.Text;
using System.Windows.Forms;

namespace Classes.CustomControls
{
public partial class CostLineReport : UserControl, IDisposable
{
    #region CONSTANTS
    public static int pnlWidth = 870;
    public static int pnlHeightBase = 112;
    public static int pnlHeightExtended = 415;
    public static int pnlHeightCollapsed = 30;
    public static Color ColorNeutral = Color.FromArgb(176, 196, 222);
    public static Color ColorApprove = Color.FromArgb(173, 255, 47);
    public static Color ColorDisapprove = Color.FromArgb(255, 99, 71);
    public static Image ImageApprove = Image.FromFile(@"Images\tableAdd.png");
    public static Image ImageDisapprove = Image.FromFile(@"Images\tableDelete.png");
    public static Image ImageDetail = Image.FromFile(@"Images\tableDetail.png");
    public static Image ImageCollapse = Image.FromFile(@"Images\compile-warning.png");
    public static Image ImageViewRecords = Image.FromFile(@"Images\table.png");
    public static Image ImageCalculationInclude = Image.FromFile(@"Images\add.png");
    public static Image ImageCalculationExclude = Image.FromFile(@"Images\delete.png");
    #endregion

    #region FIELDS
    private CostLineReportData _Costlinereportdata;
    private ToolTip ttpApprove;
    private ToolTip ttpDisapprove;
    private ToolTip ttpCollapse;
    private ToolTip ttpDetail;
    private ToolTip ttpViewRecords;
    private ToolTip ttpCalculation;
    #endregion

    #region CTORS
    public CostLineReport(CostLineReportData line)
    {
        InitializeComponent();
        //
        this._Costlinereportdata = line;
        //
        this.picApprove.Click += new EventHandler(Approve);
        this.picDisapprove.Click += new EventHandler(Disapprove);
        this.picDetail.Click += new EventHandler(ResizeControl);
        this.picCollapse.Click += new EventHandler(CollapseControl);
        this.picViewRecords.Click += new EventHandler(ShowRecords);
        this.picCalculation.Click += new EventHandler(SwitchCalculateState);
        //
        this.rtMainData.Text = _Costlinereportdata.Maintext;
        this.rtDetail.Text = _Costlinereportdata.Detailtext; ;
        this.lblTitle.Text = _Costlinereportdata.Title;
        //
        ttpApprove = new ToolTip();
        ttpDisapprove = new ToolTip();
        ttpCollapse = new ToolTip();
        ttpDetail = new ToolTip();
        ttpViewRecords = new ToolTip();
        ttpCalculation = new ToolTip();
        ttpApprove.SetToolTip(this.picApprove, "Approve this line");
        ttpDisapprove.SetToolTip(this.picDisapprove, "Disapprove this line");
        ttpCollapse.SetToolTip(this.picCollapse, "Collapse this line");
        ttpDetail.SetToolTip(this.picDetail, "Show detail");
        ttpViewRecords.SetToolTip(this.picViewRecords, "View associated recordset");
        ttpCalculation.SetToolTip(this.picCalculation, "Include/Exclude from calculation");
        //
        this.picApprove.Image = CostLineReport.ImageApprove;
        this.picDisapprove.Image = CostLineReport.ImageDisapprove;
        this.picDetail.Image = CostLineReport.ImageDetail;
        this.picCollapse.Image = CostLineReport.ImageCollapse;
        this.picViewRecords.Image = CostLineReport.ImageViewRecords;
        this.picCalculation.Image = CostLineReport.ImageCalculationExclude;
        //
        Recolor();
    }
    #endregion

    #region PROPERTIES
    public RichTextBox MainTextBox
    { get { return this.rtMainData; } }
    public RichTextBox DetailTextBox
    { get { return this.rtDetail; } }
    public Label TitleLabel
    { get { return this.lblTitle; } }
    public PictureBox CalculateControl
    { get { return this.picCalculation; } }
    #endregion

    #region METHODS
    private void Approve(object o, EventArgs e)
    {
        _Costlinereportdata.Approve();
        Recolor();
    }
    private void Disapprove(object o, EventArgs e)
    {
        _Costlinereportdata.Disapprove();
        Recolor();
    }
    private void ResizeControl(object o, EventArgs e)
    {
        _Costlinereportdata.SwitchSize();
        switch(_Costlinereportdata.Viewstate)
        {
            case ViewState.Base:
                this.Height = CostLineReport.pnlHeightBase;
                break;
            case ViewState.Extended:
                this.Height = CostLineReport.pnlHeightExtended;
                break;
        }
    }
    private void CollapseControl(object o, EventArgs e)
    {
        _Costlinereportdata.Collapse();
        if (_Costlinereportdata.Collapsed)
            this.Height = CostLineReport.pnlHeightCollapsed;
        else
            this.Height = CostLineReport.pnlHeightBase;
    }
    private void Recolor()
    {
        switch (_Costlinereportdata.Approvalstate)
        {
            case ApprovalState.Approved:
                foreach (Control c in pnlColorIndicator.Controls)
                {
                    if (c is PictureBox)
                        ((PictureBox)c).BackColor = CostLineReport.ColorApprove;
                }
                pnlColorIndicator.BackColor = CostLineReport.ColorApprove;
                break;
            case ApprovalState.Disapproved:
                foreach (Control c in pnlColorIndicator.Controls)
                {
                    if (c is PictureBox)
                        ((PictureBox)c).BackColor = CostLineReport.ColorDisapprove;
                }
                pnlColorIndicator.BackColor = CostLineReport.ColorDisapprove;
                break;
            case ApprovalState.Neutral:
                foreach (Control c in pnlColorIndicator.Controls)
                {
                    if (c is PictureBox)
                        ((PictureBox)c).BackColor = CostLineReport.ColorNeutral;
                }
                pnlColorIndicator.BackColor = CostLineReport.ColorNeutral;
                break;
        }
    }
    private void ShowRecords(object sender, EventArgs e)
    {
        if (this._Costlinereportdata.Costline.LocalData != null)
        {
            using (Forms.frmCostlineRecords f = new Forms.frmCostlineRecords(this._Costlinereportdata.Costline.LocalData))
            {
                f.ShowDialog();
            }
        }
        else
            MessageBox.Show("This line has no records associated to it. The detailed list cannot be shown.",
                            "Can't show form",
                            MessageBoxButtons.OK,
                            MessageBoxIcon.Information);
    }
    private void SwitchCalculateState(object sender, EventArgs e)
    {
        if (this._Costlinereportdata.Calculationstate == CalculationState.Included)
        {
            this._Costlinereportdata.Calculationstate = CalculationState.Excluded;
            this.picCalculation.Image = CostLineReport.ImageCalculationExclude;
        }
        else
        {
            this._Costlinereportdata.Calculationstate = CalculationState.Included;
            this.picCalculation.Image = CostLineReport.ImageCalculationInclude;
        }
    }
    public void SetCalculateState(CalculationState st)
    {
        switch (st)
        {
            case CalculationState.Included:
                this._Costlinereportdata.Calculationstate = CalculationState.Excluded;
                break;
            case CalculationState.Excluded:
                this._Costlinereportdata.Calculationstate = CalculationState.Included;
                break;
        }
        this.SwitchCalculateState(this, null);
    }
    #endregion

    #region INTERFACE_IMEPLEMENTS
    void IDisposable.Dispose()
    {
        this._Costlinereportdata = null;
        this.picApprove.Image.Dispose();
        this.picCalculation.Image.Dispose();
        this.picCollapse.Image.Dispose();
        this.picDetail.Image.Dispose();
        this.picDisapprove.Image.Dispose();
        this.picViewRecords.Image.Dispose();
        this.rtDetail.Dispose();
        this.rtMainData.Dispose();
        this.lblDivider.Dispose();
        this.lblDivider2.Dispose();
        this.lblDivider3.Dispose();
        this.lblDivider4.Dispose();
        this.lblTextDivider.Dispose();
        this.lblTitle.Dispose();
        this.picApprove.Dispose();
        this.picCalculation.Dispose();
        this.picCollapse.Dispose();
        this.picDetail.Dispose();
        this.picDisapprove.Dispose();
        this.picViewRecords.Dispose();
        this.pnlColorIndicator.Dispose();
        ttpApprove.Dispose();
        ttpDisapprove.Dispose();
        ttpCollapse.Dispose();
        ttpDetail.Dispose();
        ttpViewRecords.Dispose();
        ttpCalculation.Dispose();
        base.Dispose(true);
    }
    #endregion
}
}

This is the content (as far as winform controls go) for my user control

CostLineReport - System.Windows.Forms.UserControl
-lblDivider - System.Windows.Forms.Label
-lblDivider2 - System.Windows.Forms.Label
-lblDivider3 - System.Windows.Forms.Label
-lblDivider4 - System.Windows.Forms.Label
-lblTextDivider - System.Windows.Forms.Label
-lblTitle - System.Windows.Forms.Label
-lblTopDivider - System.Windows.Forms.Label
-picApprove - System.Windows.Forms.PictureBox
-picCalculation - System.Windows.Forms.PictureBox
-picCollapse - System.Windows.Forms.PictureBox
-picDetail - System.Windows.Forms.PictureBox
-picDisapprove - System.Windows.Forms.PictureBox
-picViewRecords - System.Windows.Forms.PictureBox
-pnlColorIndicator - System.Windows.Forms.Panel
-rtDetail - System.Windows.Forms.RichTextBox
-rtMaindata - System.Windows.Forms.RichTextBox

This is how I clear my flowlayoutpanel of all content:

while (pnlCenterRightControls.Controls.Count > 0)
{
    pnlCenterRightControls.Controls[0].Dispose();
}
GC.Collect();

This is how I add my controls

I have added this block because this also adds a Delegate (event) method to one of the internal controls in the User control.

foreach (Classes.CustomControls.CostLineReportData c in SelectedCostlineReports)
    {
        Classes.CustomControls.CostLineReport r = c.ToControl();
        r.CalculateControl.Click += delegate(object o, EventArgs e) { GetCostCalculation(); };
        this.pnlCenterRightControls.Controls.Add(r);
    }

EDIT SOLUTION
For those of you who are interested, I have updated my code to following:

Removing controls from the flowlayoutpanel
I also delete a subscriber to the event here

while (pnlCenterRightControls.Controls.Count > 0)
{
    foreach (Control contr in pnlCenterRightControls.Controls)
    {
        Classes.CustomControls.CostLineReport clrp = contr as Classes.CustomControls.CostLineReport;
        clrp.CalculateControl.Click -= GetCostCalculation;
        clrp.Dispose();
    }
}

Disposing my objects
(not disposing my static images though)

    new public void Dispose()
    {
        this.Dispose(true);
    }
    /// <summary> 
    /// Clean up any resources being used.
    /// </summary>
    /// <param name="disposing">true if managed resources should be disposed; otherwise, false.</param>
    protected override void Dispose(bool disposing)
    {
        this.picApprove.Click -= Approve;
        this.picDisapprove.Click -= Disapprove;
        this.picDetail.Click -= ResizeControl;
        this.picCollapse.Click -= CollapseControl;
        this.picViewRecords.Click -= ShowRecords;
        this.picCalculation.Click -= SwitchCalculateState;
        this._Costlinereportdata = null;
        this.rtDetail.Dispose();
        this.rtMainData.Dispose();
        this.lblDivider.Dispose();
        this.lblDivider2.Dispose();
        this.lblDivider3.Dispose();
        this.lblDivider4.Dispose();
        this.lblTextDivider.Dispose();
        this.lblTitle.Dispose();
        this.lblToplDivider.Dispose();
        this.picApprove.Dispose();
        this.picCalculation.Dispose();
        this.picCollapse.Dispose();
        this.picDetail.Dispose();
        this.picDisapprove.Dispose();
        this.picViewRecords.Dispose();
        this.pnlColorIndicator.Dispose();
        ttpApprove.Dispose();
        ttpDisapprove.Dispose();
        ttpCollapse.Dispose();
        ttpDetail.Dispose();
        ttpViewRecords.Dispose();
        ttpCalculation.Dispose();
        if (disposing && (components != null))
        {
            components.Dispose();
        }
        base.Dispose(disposing);
    }

Answer 1

C# is garbage collected so it make no sense to have so many Dispose in your code.

Usually, you don't have to care much about anything that was added by the designer but only by your own code (one should expected generated code to be correct).

Also since UserControl already implement protected override void Dispose(bool disposing)you should really override that method instead of trying to re-implement the interface.

I don't understand how your code could even works... as your code probably call Dispose multiple times on the same object. I guess that predefined controls ignore the second call as otherwise you would have exception because an object is already disposed.

Any control added by the designer will be automatically disposed.

Calling Dispose on an inner object is absolute total nonsense. Each object should dispose its own objects. Always. Period.

picApprove.Image.Dispose();    // BAD CODE!!!

Obviously, when you manually add a control and manually connect an event handler, you generally have to manually remove both. Although your syntax is probably equivalent, I would recommend you to use the same short syntax when you add an event that when you remove it (except += instead of -=). That way, it is easier to ensure that they both matches. Use:

r.CalculateControl.Click += GetCostCalculation;

instead of:

r.CalculateControl.Click += 
    delegate(object o, EventArgs e) { GetCostCalculation(); };

And why are you manually adding event handlers for controls that are added by the designer? I don't see any advantage of initializing part of the control in the designer and other part in the constructor. You will just increase the risk of regression bug if someone else maintain that code and think that the event was not properly connected.

new public void Dispose() is another complete non sense.

Finally, I see many other problem with your code... It does not follows many good design practices. I would recommend you to learn about SOLID design patterns.

A few examples of poor practices:

• Public properties that return internal controls. It should generally not be necessary as the code that operate on those control should reside in this class. You might search information on TDA principle too.
• Tight coupling between UI and business code.
• Duplicate code (DRY principle) in Recolor function.
• Nomenclature that does not respect .NET convention particularly for event handler function name.

Answer 2

It is hard to tell without having the code available to run through a profiler but one thing I do notice is you are not -= your events. Any handlers for your events will still contain a reference to your objects and as such will make them ineligible for garbage collection.

EDIT:

There are two sets of events to worry about, those you have internally in your control (which you will need to manually remove in your dispose method) and those external ones which reference you control or it's children.

Any automatic way I have seen to do this is buggy or does not work great, in general I find it best to do it manually. See this answer for more info on auto removing events.

How to remove all event handlers from a control

Answer 3

From what it looks like you should dispose of all of these objects as well:

ImageApprove.Dispose;
ImageDisapprove.Dispose;
ImageDetail.Dispose;
ImageCollapse.Dispose;
ImageViewRecords.Dispose;
ImageCalculationInclude.Dispose;
ImageCalculationExclude.Dispose;