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

Compose Only Based Navigation: View Model Leaks Not Auto Detected? #2678

Open
ryanholden8 opened this issue May 17, 2024 · 5 comments
Open

Comments

@ryanholden8
Copy link

We have an app with Leak Canary integrated within our instrumentation tests. Works great for activity leaks, as we manually created a leak and it fails the test. Auto detection of activities working.

However, we manually created a leaking view model (a strong global reference is held on to the view model within its init) and the test continues to pass. Auto detection of view models not working.

  • The app is a single activity where all the navigation is done within compose. No activity/fragment navigation.
  • The view model store owner is NavControllerViewModel from androidx.navigation. So I'm not sure it's tied to any fragment/activity lifecycles callbacks? Which I'm assuming is auto the auto detection of view model is based?
  • This store is provided at the time of view model creation within compose code using LocalViewModelStoreOwner.current provided by NavBackStackEntry.LocalOwnersProvider here.

Is there additional setup that is needed to detect view models created from Compose when using Compose Navigation instead of Activity/Fragment navigation? The docs say no.
Is there documentation on how this auto view model detection works?

@Dimezis
Copy link

Dimezis commented May 20, 2024

So what are you doing exactly to trigger the leak? Configuration change?
Just storing the reference is not enough if the corresponding nav controller is still alive.

@ryanholden8
Copy link
Author

ryanholden8 commented May 20, 2024

So what are you doing exactly to trigger the leak detection? Configuration change? Just storing the reference is not enough if the corresponding nav controller is still alive.

NavControllerViewModel seems to release/clear view models correctly as things navigate around but it's an internal implementation class we can only see while debugging. It's a navigation view model that has its own view model store for view models within its navigation scope. It's a parent view model and it is a view model store for its children view models. Each navigation stack item/screen is a child view model connected to NavControllerViewModel, it seems from our debugging. We have also asserted, manually, that NavControllerViewModel itself is cleaned up when the test finishes by adding it to objects to watch. However, NavControllerViewModel's view model store for it's children view model is a private property we can't access.

Keeping a reference around for our view model when NavControllerViewModel clears it and it itself is cleaned up would causes a leak if auto leak detection for view models worked, no?

@pyricau
Copy link
Member

pyricau commented May 21, 2024

I haven't tested view model leak detection on compose projects.

View model leak detection is kicked off by leakcanary.internal.AndroidXFragmentDestroyWatcher, in 2 places:

  1. On LeakCanary init, if androidx.fragment.app.Fragment is in the classpath, AND the activity is a FragmentActivity, then we hook into the activity view model store
  2. On fragment creation, we hook into the fragment's view model store.

I suspect that the issue here is one of:

  • NavControllerViewModel isn't tied to an activity or fragment view model store (?). Not sure exactly how this is setup.
  • Your activity isn't a FragmentActivity
  • You don't have the androidx.fragment.app.Fragment class in your classpath.

I wrote all this before Compose was a thing, so it's very possible we have a bug. Let me know what you find. Don't hesitate to put breakpoints in leakcanary.internal.AndroidXFragmentDestroyWatcher.invoke() and leakcanary.internal.ViewModelClearedWatcher.install() to figure out what's up.

@ryanholden8
Copy link
Author

I haven't tested view model leak detection on compose projects.

View model leak detection is kicked off by leakcanary.internal.AndroidXFragmentDestroyWatcher, in 2 places:

  1. On LeakCanary init, if androidx.fragment.app.Fragment is in the classpath, AND the activity is a FragmentActivity, then we hook into the activity view model store
  2. On fragment creation, we hook into the fragment's view model store.

I suspect that the issue here is one of:

  • NavControllerViewModel isn't tied to an activity or fragment view model store (?). Not sure exactly how this is setup.
  • Your activity isn't a FragmentActivity
  • You don't have the androidx.fragment.app.Fragment class in your classpath.

I wrote all this before Compose was a thing, so it's very possible we have a bug. Let me know what you find. Don't hesitate to put breakpoints in leakcanary.internal.AndroidXFragmentDestroyWatcher.invoke() and leakcanary.internal.ViewModelClearedWatcher.install() to figure out what's up.

Thank you for this info! This allows me to investigate further..

@pyricau
Copy link
Member

pyricau commented May 30, 2024

Cool. Did you find anything?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants