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

Fixed memory leaks in ContextMenu.cs #12526

Merged
merged 5 commits into from
Aug 15, 2023

Conversation

adirh3
Copy link
Contributor

@adirh3 adirh3 commented Aug 12, 2023

What does the pull request do?

Fixes two memory leaks -

  1. Leak when controls are not freed when they have any ContextMenu set.
  2. Leak when ItemPanel is set to StackPanel;' I don't know why.

What is the current behavior?

Two issues:

  1. When you have a Control with ContextMenu applied, for example from a ControlTheme, there will be only 1 ContextMenu created for all controls of these type, but each time a control is created it will be added to _attachedControls and will stay there forever.
  2. For some reason, when a ContextMenu with ItemsSource binding have an ItemsPanel as StackPanel the items created will not be cleared. When changing ItemsPanel to VirtualizingStackPanel there is no issue.

What is the updated/expected behavior with this PR?

No memory leaks when using ContextMenu

How was the solution implemented (if it's not obvious)?

  1. Add and remove attached controls when they attach and detach from visual tree.
  2. Replace override of default ItemsPanel, so it uses VirtualizingStackPanel like every other place. I saw that this line was from 2019, and I can't think of any particular reason why it needs to stay.

Note - the issue when using StackPanel is not resolved, but I didn't have time to investigate it.

Checklist

  • Added unit tests (if possible)? I added but can't test it, all DotMemory unit tests fail for some reason
  • Added XML documentation to any related classes?
  • Consider submitting a PR to https://github.com/AvaloniaUI/Documentation with user documentation

Breaking changes

Obsoletions / Deprecations

Fixed issues

@avaloniaui-team
Copy link
Contributor

You can test this PR using the following package version. 11.0.999-cibuild0038437-beta. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@maxkatz6 maxkatz6 added this pull request to the merge queue Aug 15, 2023
Merged via the queue into AvaloniaUI:master with commit fa6bcc9 Aug 15, 2023
5 checks passed
@maxkatz6 maxkatz6 added backport-candidate-11.0.x Consider this PR for backporting to 11.0 branch backported-11.0.x and removed backport-candidate-11.0.x Consider this PR for backporting to 11.0 branch labels Dec 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants