List<T>.Clear - Does it have to be called?

Question

So I've been fighting another memory problem in my project for the past week. I tried a couple of memory profilers but nothing gave me insight into what was causing the minor memory leak. The following code turned out to be causing it:

private void DeleteAll( FlowLayoutPanel flp)
{
    List<ImageControl> AllList = GetAllList(flp);
    List<ImageControl> LockedList = GetLockedList(flp);

    for (int i = 0; i < LockedList.Count; i++)
    {
        AllList.Remove(LockedList[i]);
    }

    flp.SuspendLayout();

    for (int i = 0; i < AllList.Count; i++)
    {
        flp.Controls.Remove(AllList[i]);
    }

    DisposeList(AllList);

    flp.ResumeLayout();

}

In the code, ImageControl is a UserControl, and the entire method above just removes ImageControls from a FlowLayoutPanel. The DisposList() method just calls ImageControl.Dispose() for all the controls in the list passed to it.

Now, I thought that once this method had exited, AllList would be out of scope and hence all its references to the ImageControl's would be nonexistent. So the GC would do it's stuff. But it wasn't. I found it requires

AllList.Clear();

added to the end of the DeleteAll() method, before AllList was out of scope.

So do you have to always explicitly clear a generic list to free up resources? Or is it something I'm doing wrong above? I'd like to know since I'm making fairly heavy use of temporary Lists in this project.

Ok, here's the GetAllList method. Doesn't look like a problem to me:

private List<ImageControl> GetAllList(FlowLayoutPanel flp)
{
    List<ImageControl> List = new List<ImageControl>();

    for (int i = 0; i < flp.Controls.Count; i++)
    {
        List.Add((ImageControl)flp.Controls[i]);
    }

    return List;
}

BTW, if you see my last couple of topics here I've been fighting memory leaks in my quest to become a proficient c# programmer :) I added the DisposeList() method since I've read Dispose() should be called on any object that implements IDisposable, which UserControl does. I also needed a way to fix up a "bug" with the ToolStrip class (which ImageControl contains), where it causes resources to remain unless the Visible property is set to false before it's destroyed. So I've overridden the Dispose method of ImageControl to do just that.

Oh, and DisposeList() also unsubscribes from an event handler:

private void DisposeList( List<ImageControl> IC )
{
    for (int i=0;i<IC.Count;i++)
    {
        IC[i].DoEvent -= ImageButtonClick;
        IC[i].Dispose();
    }
}

Answer 1

If AllList were the only reference to the list and the elements in the list, then the list and all its elements would become eligible for garbage collection as soon as you exit the DeleteAll method.

If calling AllList.Clear() makes a difference, then I would conclude that there is a reference to the same list being held elsewhere in your code. Maybe a closer look at the GetAllList() method would give a clue where.

Answer 2

You shouldn't have to clear the list. Can you share your GetAllList() function? The fact that you even need a corresponding "DisposeList()" method tells me there are probably side effects there that keep a reference to your list somewhere.

Also, I'd simplify that code like this:

private void DeleteAll( FlowLayoutPanel flp)
{
    var UnlockedImages = flp.Controls.OfType<ImageControl>().Except(GetLockedList(flp));

    flp.SuspendLayout();

    foreach (ImageControl ic in UnlockedImages)
    {
        flp.Controls.Remove(ic);
    }

    flp.ResumeLayout();
}