Skip to content

[windows] fix memory leak in TabbedPage#23281

Merged
PureWeen merged 1 commit intodotnet:mainfrom
jonathanpeppers:WindowsTabbedPageLeaks
Jul 2, 2024
Merged

[windows] fix memory leak in TabbedPage#23281
PureWeen merged 1 commit intodotnet:mainfrom
jonathanpeppers:WindowsTabbedPageLeaks

Conversation

@jonathanpeppers
Copy link
Copy Markdown
Member

@jonathanpeppers jonathanpeppers commented Jun 26, 2024

Context: #23166

This expands upon #23166 (iOS/Catalyst memory leaks for TabbedPage),
fixing the same problem on Windows. I could reproduce the problem in
MemoryTests.cs.

I found the NavigationViewItemViewModel.Data property held the
ContentPage that held onto the TabbedPage. I made the backing field
a WeakReference and the test in MemoryTests.cs now passes on Windows.

@jonathanpeppers jonathanpeppers added the perf/memory-leak 💦 Memory usage grows / objects live forever (sub: perf) label Jun 26, 2024
@jonathanpeppers
Copy link
Copy Markdown
Member Author

Update: I just reverted my changes related to the events, and it seems like maybe only NavigationViewItemViewModel.Data is a problem?

Will know for sure when CI is working again.

@jonathanpeppers jonathanpeppers force-pushed the WindowsTabbedPageLeaks branch 2 times, most recently from 2b9de8b to 75325f1 Compare June 28, 2024 13:37
@jonathanpeppers jonathanpeppers marked this pull request as ready for review June 28, 2024 13:37
@jonathanpeppers jonathanpeppers requested a review from a team as a code owner June 28, 2024 13:37
Context: dotnet#23166

This expands upon dotnet#23166 (iOS/Catalyst memory leaks for `TabbedPage`),
fixing the same problem on Windows. I could reproduce the problem in
`MemoryTests.cs`.

I found the `NavigationViewItemViewModel.Data` property held the
`ContentPage` that held onto the `TabbedPage`. I made the backing field
a `WeakReference` and the test in `MemoryTests.cs` now passes on Windows.
@jonathanpeppers jonathanpeppers force-pushed the WindowsTabbedPageLeaks branch from 75325f1 to 8bd1226 Compare June 28, 2024 18:05
@jonathanpeppers
Copy link
Copy Markdown
Member Author

/azp run

@azure-pipelines
Copy link
Copy Markdown

Azure Pipelines successfully started running 3 pipeline(s).

@PureWeen PureWeen merged commit 01408ca into dotnet:main Jul 2, 2024
@jonathanpeppers jonathanpeppers deleted the WindowsTabbedPageLeaks branch July 2, 2024 19:20
@github-actions github-actions bot locked and limited conversation to collaborators Aug 2, 2024
@samhouts samhouts added fixed-in-8.0.70 fixed-in-net9.0-nightly This may be available in a nightly release! labels Aug 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-controls-tabbedpage TabbedPage fixed-in-8.0.70 fixed-in-net9.0-nightly This may be available in a nightly release! perf/memory-leak 💦 Memory usage grows / objects live forever (sub: perf) platform/windows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants