KEMBAR78
[iOS] Fix memory leak in CarouselViewHandler2 by bhavanesh2001 · Pull Request #29719 · dotnet/maui · GitHub
Skip to content

Conversation

@bhavanesh2001
Copy link
Contributor

@bhavanesh2001 bhavanesh2001 commented May 28, 2025

Note

Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!

Description of Change

CarouselViewHandler2 sets a VisibleItemsInvalidationHandler on the NSCollectionLayoutSection during layout construction. This handler captures strong references to VirtualView and Controller, causing a memory leak.

Updated the handler to use WeakReference's. Moved Layout creation to LayoutFacotry

Befor fix After fix
Image Screenshot 2025-05-29 at 12 56 59 AM

Issues Fixed

Fixes #29677

@bhavanesh2001 bhavanesh2001 requested a review from a team as a code owner May 28, 2025 19:50
@dotnet-policy-service dotnet-policy-service bot added the community ✨ Community Contribution label May 28, 2025
@dotnet-policy-service
Copy link
Contributor

Hey there @@bhavanesh2001! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

@bhavanesh2001 bhavanesh2001 marked this pull request as draft May 28, 2025 20:00
@bhavanesh2001 bhavanesh2001 marked this pull request as ready for review May 28, 2025 20:02
@jsuarezruiz jsuarezruiz added platform/ios area-controls-collectionview CollectionView, CarouselView, IndicatorView perf/memory-leak 💦 Memory usage grows / objects live forever (sub: perf) labels May 29, 2025
@jsuarezruiz
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@bhavanesh2001 bhavanesh2001 requested a review from rmarinho May 29, 2025 11:16
rmarinho
rmarinho previously approved these changes May 29, 2025
@rmarinho rmarinho added this to the .NET 9 SR8 milestone May 29, 2025
@rmarinho rmarinho moved this from Todo to Approved in MAUI SDK Ongoing May 29, 2025
@rmarinho rmarinho changed the base branch from main to inflight/current May 29, 2025 11:34
@rmarinho rmarinho changed the base branch from inflight/current to inflight/candidate May 29, 2025 11:34
@rmarinho rmarinho changed the base branch from inflight/candidate to main May 29, 2025 11:35
@rmarinho rmarinho dismissed their stale review May 29, 2025 11:35

The base branch was changed.

rmarinho
rmarinho previously approved these changes May 29, 2025
@rmarinho rmarinho changed the base branch from main to inflight/candidate May 30, 2025 16:12
@rmarinho rmarinho changed the base branch from inflight/candidate to inflight/current May 30, 2025 16:12
@rmarinho rmarinho changed the base branch from inflight/current to main May 30, 2025 16:13
@rmarinho rmarinho dismissed their stale review May 30, 2025 16:13

The base branch was changed.

@rmarinho rmarinho merged commit 6b41f4f into dotnet:main May 30, 2025
131 checks passed
@github-project-automation github-project-automation bot moved this from Approved to Done in MAUI SDK Ongoing May 30, 2025
@sheiksyedm
Copy link
Contributor

The CarouselView tests failed on CI due to the changes introduced by this PR. Setting the FlowDirection to either LeftToRight or RightToLeft on CarouselView is now failing.
https://dev.azure.com/xamarin/public/_build/results?buildId=143228&view=ms.vss-test-web.build-test-results-tab
fyi: @bhavanesh2001

@bhavanesh2001
Copy link
Contributor Author

@sheiksyedm Thanks for letting me know. #29784 should fix the failures

@github-actions github-actions bot locked and limited conversation to collaborators Jul 3, 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 community ✨ Community Contribution perf/memory-leak 💦 Memory usage grows / objects live forever (sub: perf) platform/ios

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

CarouselViewHandler2 causes a memory leak

4 participants