45

I have a basic knowledge of memory leaks and what can cause them. That's why I don't understand if I have a problem in my code or is it a false positive. I don't know which part of the code I should share since the project is not small. But just let me know in the comments and I will add required code.

I use navigation arch component and follow MVVM pattern. I added LeakCanary library later in the development of project and it immediately started to give me warnings about retained instances when I navigate between screens.

The problem occurs when I add fragments to the back stack. With each added fragment to the back stack the counter of retained instances increases. When it reaches the threshold value of 5 LeakCanary dumps the heap and provides report.

But if I click on back button and return to previous screens then counter of retained instances decreases and eventually, when returned to 1st screen all retained instances disappear.

If I look at heap analysis reports it says that the variable coordinatorLayout which is a reference to the CoordinatorLayout in xml has leaked. If I remove the variable and all of its usage and run the app again I see the same problem, but now with another variable that is a reference to another view in xml. I tried to remove all of the views and their usage that LeakCanary reported as leaking. When it said that a TextView, which is just used to set a text in onViewCreated and not used anywhere else, is leaking I started to doubt that there is a problem in my code.

I analyzed the lifecycle method calls in fragments and noticed that when I navigate to new screen for previous fragment all methods till and including onDestroyView gets called but not onDestroy. When I click back onDestroy is called for fragment that was on top of back stack and retained instances counter decreases.

I suspect that Navigation component is keeping the instance of a fragment when it is in back stack and LeakCanary is seeing it as a leak.

Marat
  • 6,142
  • 6
  • 39
  • 67

1 Answers1

84

That's how Fragments on the back stack work (and Navigation just uses the existing Fragment APIs): the Fragment's view is destroyed, but the Fragment itself is not destroyed - they are kept in the CREATED state until you hit the back button and return to the Fragment (after which onCreateView() will be called again and you'll move back up to RESUMED).

As per the Fragments: Past, Present, and Future talk, one of the future changes coming to Fragments is an opt in option to destroy Fragments on the back stack, rather than having two separate lifecycles. This isn't available as of yet.

You have to null out your references to the views in onDestroyView as that's the sign that the view is no longer being used by the Fragment system and it can be safely garbage collected if it wasn't for your continued reference to the View.

ianhanniballake
  • 191,609
  • 30
  • 470
  • 443
  • 8
    Does Android View Binding solve this problem? I can't find any documentation on whether the reference to View Binding views (maybe the binding object itself) is automatically 'nulled out' in `onDestroyView` with View Binding. – Tim Malseed Jan 09 '20 at 13:52
  • 7
    @TimMalseed - you need to null out your reference to the binding object yourself, there's nothing automatic going on. – ianhanniballake Jan 09 '20 at 14:04
  • @ianhanniballake So, we should NEVER use `lateinit var view:View` in `Fragment` with Kotlin ? – Pauland Apr 03 '20 at 10:39
  • 2
    @Pauland - that's always been the case, yes. It'll continue to be the case until the changes referenced in the talk I linked in the answer are made. – ianhanniballake Apr 03 '20 at 14:05
  • 1
    If using DataBinding, is this solved if we set the owner of the binding to `viewLifecycleOwner`? The binding should be cleared automatically, right? – Emmanuel Apr 09 '20 at 13:50
  • 3
    @Emmanuel - you need to drop your reference to the binding object itself as that holds a hard reference to the Views it owns. – ianhanniballake Apr 09 '20 at 13:52
  • Wow. Good to know. I would have thought that the binding should be aware of its owner and dispose of the internal references when it is not needed anymore. Does that make sense?Could that be a change that is made in future releases of DataBinding? – Emmanuel Apr 09 '20 at 14:01
  • 1
    @Emmanuel - you can always [file a feature request](https://issuetracker.google.com/issues/new?component=192721)! – ianhanniballake Apr 09 '20 at 14:31
  • Will do. Do you think what I am proposing is feasible? Since the binding has an instance of the owner, it could know when to null out the internal references of the views it holds, right? I want to make sure that if I file a feature request that it is feasible and makes sense. – Emmanuel Apr 09 '20 at 14:36
  • 1
    @Emmanuel - I think it would certainly be a change in behavior (which may imply it being a separate opt in flag), but having the correct LifecycleOwner would be enough information for it to fix a whole swath of memory issues. – ianhanniballake Apr 09 '20 at 14:38
  • Feature [request](https://issuetracker.google.com/issues/153613055) sent. Thanks a lot @ianhanniballake. – Emmanuel Apr 09 '20 at 14:57
  • Wow, I've been struggling with a similar issue and TIL. Holy moly, I had no idea. Thanks a lot you guys! – lawonga Sep 19 '20 at 04:56