Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ToolStripMenuItem can lead to memory leaks #4808

Open
kirsan31 opened this issue Apr 18, 2021 · 4 comments · May be fixed by #11358
Open

ToolStripMenuItem can lead to memory leaks #4808

kirsan31 opened this issue Apr 18, 2021 · 4 comments · May be fixed by #11358
Labels
area-Controls-StripControls 🚧 work in progress Work that is current in progress help wanted Good issue for external contributors tenet-performance Improve performance, flag performance regressions across core releases
Milestone

Comments

@kirsan31
Copy link
Contributor

  • .NET Core Version: all up to latest (5.0.5)

  • Have you experienced this same bug with .NET Framework?: Yes

Problem description:
ToolStripMenuItem can lead to memory leaks in some special conditions.

  • Menu with submenu.
 windowsToolStripMenuItem 
  -> listToolStripMenuItem
  • Populate and clear submenu in runtime. In fact, only cleaning is important.
private void WindowsToolStripMenuItem_DropDownOpened(object sender, EventArgs e)
{
    for (int i = 0; i < 4; i++)
        listToolStripMenuItem.DropDownItems.Add("MenuItem" + i);
}

private void windowsToolStripMenuItem_DropDownClosed(object sender, EventArgs e)
{
    while (listToolStripMenuItem.DropDownItems.Count > 0)
        listToolStripMenuItem.DropDownItems[listToolStripMenuItem.DropDownItems.Count - 1].Dispose();
}

In certain conditions (I think that is - open only main menu and not opening submenu listToolStripMenuItem) after closing main menu some of listToolStripMenuItem subitems will remain in memory. See video below.
This happens due to _displayedItems collection still have live references to removed menu items. I think this collection ( and may be _overflowItems too ) must be checked on item removal.

menu.mp4

Workaround:
You can call listToolStripMenuItem.DropDown.PerformLayout(); after items removal. This lead to call of SetDisplayedItems() which will clear _displayedItems of removed elements.

Expected behavior:
All removed and disposed menu elements must be eligible to GC (have no live references).

Minimal repro:
WinFormsCoreTest_Grid_Menu.zip

@RussKie RussKie added this to the Future milestone Apr 21, 2021
@RussKie RussKie added tenet-performance Improve performance, flag performance regressions across core releases help wanted Good issue for external contributors labels Apr 21, 2021
@Tanya-Solyanik
Copy link
Member

@Olina-Zhang - could you please retest this issue?

@Olina-Zhang
Copy link
Member

@Olina-Zhang - could you please retest this issue?

Verified this issue in the latest .Net 8.0 build as above sample application and video in VS, have same testing result, so still repro.
image

@elachlan
Copy link
Contributor

Dispose has a section to dispose of the items. I guess we just do the same for the DisplayedItems and OverflowItems

if (!Items.IsReadOnly)
{
// only dispose the items we actually own.
for (int i = Items.Count - 1; i >= 0; i--)
{
Items[i].Dispose();
}
Items.Clear();
}

@elachlan
Copy link
Contributor

We might also want to look at OnItemRemovedInternal. Which is called by ToolStripItemCollection when an item is removed (such as when ToolStripItem.Dispose is called).

internal void OnItemRemovedInternal(ToolStripItem item)
{
KeyboardToolTipStateMachine.Instance.Unhook(item, ToolTip);
}

We could then trigger a cleanup DisplayedItems and OverflowItems, I think ToolStripItemCollection might need to pass itself to OnItemRemovedInternal so we can cleanup based on which list had the item removed.

kirsan31 added a commit to kirsan31/winforms that referenced this issue May 12, 2024
@dotnet-policy-service dotnet-policy-service bot added the 🚧 work in progress Work that is current in progress label May 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Controls-StripControls 🚧 work in progress Work that is current in progress help wanted Good issue for external contributors tenet-performance Improve performance, flag performance regressions across core releases
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants