How do I fix a memory leak caused by an object ref

问题:

I have used dotMemory to locate a memory leak. The object I want gone is referenced by an event handler through a ToolStripMenuItem and ContextMenuStrip. The object contains these properties:

    public override ContextMenuStrip PopupMenu
    {
        get
        {
            ContextMenuStrip myPopup = new ContextMenuStrip();
            myPopup.Items.Add(ItemDelete);
            return myPopup;
        }
    }

    public ToolStripMenuItem ItemDelete
    {
        get
        {
            ToolStripMenuItem itemDelete = new ToolStripMenuItem("Delete " + name);
            itemDelete.Enabled = Deletable;
            itemDelete.Image = Properties.Resources.del;
            itemDelete.Click += ItemDelete_Click;
            return itemDelete;
        }
    }

I have simplified the code, the popup menu has about a dozen menu items, which all seem to be holding on to this object after I use the popup menu to delete the object. I have tried overriding the base delete method for the object to remove the handlers, but that did not work.

    public override void delete()
    {
        if (PopupMenu != null)
        {
            ItemDelete.Click -= ItemDelete_Click;
        }

        base.delete();
    }

回答1:

Without a good, minimal, complete code example, it's impossible to know for certain what the problem is, never mind the best fix. That said…

Based on the tiny amount of code you've posted so far, it appears that you've misused the property feature in C#, and in doing so have obfuscated the code enough that you have been unable to recognize the bug. In particular, your ItemDelete property is creating a new object every time you call it, which is a horrible way to implement a getter of this kind. For the code to work correctly, you'd have to be careful to only ever call that property getter once, and manually cache the result somewhere else.

Creating a new object in a property getter isn't inherently bad, but it should be done only if that object will be cached by the property getter itself and reused for subsequent calls, or if that object is semantically a simple value (preferably an actual value type, but a simple, ummutable reference type would be fine too), where a newly created object is functionally identical to any previously created object (i.e. they can be used interchangeably without affecting the correctness of the code).

Given the above, it is possible that you would find the following alternative a useful way to fix the problem:

private Lazy<ToolStripMenuItem> _itemDelete =
    new Lazy<ToolStripMenuItem>(() => _CreateItemDelete());

private ToolStripMenuItem _CreateItemDelete()
{
    ToolStripMenuItem itemDelete = new ToolStripMenuItem("Delete " + name);
    itemDelete.Enabled = Deletable;
    itemDelete.Image = Properties.Resources.del;
    itemDelete.Click += ItemDelete_Click;
    return itemDelete;
}

public ToolStripMenuItem ItemDelete
{
    get
    {
        return _itemDelete.Value;
    }
}

This will defer creation of the ToolStripMenuItem object until the first time that the property getter is called, but will on subsequent calls to the getter return that object created on the first call.

In this way, you will ensure that when you execute the statement ItemDelete.Click -= ItemDelete_Click; later, you are actually removing the event handler from the original object, rather than some new object you created at that time.