Skip to content

[Windows] Fix memory leak with GroupedItemTemplateCollection#23386

Merged
rmarinho merged 7 commits intomainfrom
foda/GroupItemListLeak
Dec 10, 2024
Merged

[Windows] Fix memory leak with GroupedItemTemplateCollection#23386
rmarinho merged 7 commits intomainfrom
foda/GroupItemListLeak

Conversation

@Foda
Copy link
Copy Markdown
Contributor

@Foda Foda commented Jul 1, 2024

Description of Change

Fix missing event cleanup in GroupedItemTemplateCollection to match the cleanup pattern in ObservableItemTemplateCollection. This fixed a minor memory leak with grouped item collections on Windows

Issues Fixed

Fixes #22954

@Foda Foda added platform/windows area-controls-collectionview CollectionView, CarouselView, IndicatorView labels Jul 1, 2024
@Foda Foda requested review from PureWeen and jonathanpeppers July 1, 2024 16:48
@Foda Foda requested a review from a team as a code owner July 1, 2024 16:48
@Foda Foda requested a review from mattleibow July 1, 2024 16:48
@Foda
Copy link
Copy Markdown
Contributor Author

Foda commented Jul 10, 2024

/rebase

@github-actions github-actions bot force-pushed the foda/GroupItemListLeak branch from 8ec09aa to 40daa0b Compare July 10, 2024 22:29
@Foda
Copy link
Copy Markdown
Contributor Author

Foda commented Aug 19, 2024

/rebase

@Foda
Copy link
Copy Markdown
Contributor Author

Foda commented Aug 22, 2024

/azp

@azure-pipelines
Copy link
Copy Markdown

Supported commands
  • help:
    • Get descriptions, examples and documentation about supported commands
    • Example: help "command_name"
  • list:
    • List all pipelines for this repository using a comment.
    • Example: "list"
  • run:
    • Run all pipelines or specific pipelines for this repository using a comment. Use this command by itself to trigger all related pipelines, or specify specific pipelines to run.
    • Example: "run" or "run pipeline_name, pipeline_name, pipeline_name"
  • where:
    • Report back the Azure DevOps orgs that are related to this repository and org
    • Example: "where"

See additional documentation.

@Foda
Copy link
Copy Markdown
Contributor Author

Foda commented Aug 22, 2024

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 3 pipeline(s).

readonly WeakNotifyCollectionChangedProxy _proxy = new();
bool _disposedValue;

~GroupedItemTemplateCollection() => _proxy.Unsubscribe();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On Dispose you do a null check with ? operator... Wouldn't make sense to do it here as well?

@Foda
Copy link
Copy Markdown
Contributor Author

Foda commented Sep 23, 2024

/rebase

1 similar comment
@jsuarezruiz
Copy link
Copy Markdown
Contributor

/rebase

Comment on lines +333 to +335
await Task.Yield();
GC.Collect();
GC.WaitForPendingFinalizers();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a helper method that does it more robustly, is there not?

This one

public static async Task<bool> WaitForCollect(this WeakReference reference)
, I believe.

@Foda
Copy link
Copy Markdown
Contributor Author

Foda commented Nov 14, 2024

/rebase

@rmarinho
Copy link
Copy Markdown
Member

/rebase

@rmarinho rmarinho merged commit e18a175 into main Dec 10, 2024
@rmarinho rmarinho deleted the foda/GroupItemListLeak branch December 10, 2024 10:24
@rmarinho rmarinho added this to the .NET 9 SR3 milestone Dec 10, 2024
@samhouts samhouts added fixed-in-net9.0-nightly This may be available in a nightly release! fixed-in-net8.0-nightly This may be available in a nightly release! labels Dec 16, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Jan 17, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-controls-collectionview CollectionView, CarouselView, IndicatorView fixed-in-net8.0-nightly This may be available in a nightly release! fixed-in-net9.0-nightly This may be available in a nightly release! platform/windows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Windows] GroupedItemTemplateCollection CollectionChanged event not unsubscribed, causes memory leak.

7 participants