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

[android] fix memory leak with CarouselView #18584

Merged
merged 1 commit into from
Nov 8, 2023

Commits on Nov 7, 2023

  1. [android] fix memory leak with CarouselView

    Fixes: dotnet#17726
    
    Adding `CarouselView` to a page and navigating away from it keeps the
    page alive forever on Android. I was able to reproduce this in a test.
    
    It took me a while to actually track this one down, but the problem
    boils down to `MauiCarouselRecyclerView` doing:
    
        _carouselViewLayoutListener = new CarouselViewwOnGlobalLayoutListener();
        _carouselViewLayoutListener.LayoutReady += LayoutReady;
        ViewTreeObserver.AddOnGlobalLayoutListener(_carouselViewLayoutListener);
    
    The `ViewTreeObserver` lives longer than the page, and so:
    
    * `ViewTreeObserver` -> `CarouselViewwOnGlobalLayoutListener`
    * `event LayoutReady` -> `MauiCarouselRecyclerView`
    * `MauiCarouselRecyclerView` -> `CarouselView`
    * `CarouselView` -> `Parent.Parent.Parent.*` -> `Page`
    
    Thus the `Page` lives forever.
    
    If we remove the `LayoutReady` event, instead:
    
    * `CarouselViewwOnGlobalLayoutListener` saves `MauiCarouselRecyclerView`
    in a weak reference.
    
    * It can just call `LayoutReady()` directly.
    
    * `MauiCarouselRecyclerView` is able to be GC'd now.
    
    * `MauiCarouselRecyclerView.Dispose()` already appropriately will call
      `ClearLayoutListener()`
    
    * Lastly, `ViewTreeObserver?.RemoveOnGlobalLayoutListener(_carouselViewLayoutListener)`
    
    With these changes, the test passes and I don't think anything will leak
    now. :)
    
    Other changes:
    
    * I fixed the typo in `CarouselViewwOnGlobalLayoutListener`.
    * I sorted the type names in the test alphabetically.
    jonathanpeppers committed Nov 7, 2023
    Configuration menu
    Copy the full SHA
    d083d09 View commit details
    Browse the repository at this point in the history