Unregister Glide in onDestroyView

[frel]

Glide Version: 3.7.0

Integration libraries: Default.

Device/Android Version: Nexus 5, 6.0.1

Issue details / Repro steps / Use case background:

Hi,

I use Glide with fragments - Glide.with(fragment) - and I have an issue when navigating in the app. The fragments are added to the back stack and onDestroyView() is called. But onDestroy() is not called.

Before using Glide this was not a problem as we set the view references to null in onDestroyView(). But with Glide the views are still retained and I believe it is because Glide releases in onDestroy(). But the fragments are not destroyed when added to the back stack, thus resulting in a memory build up.

• Is there a way to unregister explicitly like what is done in onDestroy() in the library?
• Is there another way I can approach this?

Calling Glide.clear() on all the views is not an option as it would add way too much code.

Thank you very much

[TWiStErRob]

Glide.with(this).onDestroy();

is the only thing I can think of right now.

[TWiStErRob]

I'm not sure how Glide will react when you go back to the fragment. It should re-load all the images, because you likely load them after onCreateView. Give it a try and let's see.

[frel]

Ok, I will try to call onDestroy like you suggest. I cannot test it now, but I am setting up the views in onCreateView so that sounds promising. Thanks for the quick reply!

[frel]

This seems to work nicely. Will you consider moving this behaviour to onDestroyView instead of onDestroy? It seems to me that this is a scenario that has not been considered. Thanks again.

[TWiStErRob]

I'm pretty sure @sjudd considered this. Have you @sjudd?

Quickly, I don't think we can, because:

1. The tracker Fragment (that follows the lifecycle) doesn't have a view
2. Your onDestroyView doesn't necessarily mean that any children's view will be destroyed as well (not sure)

[perezfer]

Hi @TWiStErRob I am also experiencing the same issue as @frel. I need to release the memory used by glide by some fragments that are backstacked but not destroyed and for that reason I need to callGlide.with(this).onDestroy() on onDestroyView. The main problem I have is that I get a caught IllegalStateException that, for some reason causes the RequestManagerRetriever maintaining a reference to the fragment and thus to the Activity. What we have found out is that it is possible to overcome this issue if you call first Glide.with(this).onDestroy just before the activity is destroyed.

Here is a test project that shows how to reproduce the error and how to "fix" it. The actual fix might be to remove the reference to the activity or fragment from inside the catch.

https://github.com/perezfer/abomination/tree/glide-leak

[sjudd]

The documentation seems to indicate that onDestroyView is actually called, even if we return a null View from onCreateView. Seems like a good target for a PR. I'm not sure what's going on with @perezfer's IllegalStateException though, maybe provide a stack trace or open a new issue?

[electrolobzik]

Let me share my thoughts about this. After digging into the code I have 2 ideas:

1. Add new separate method RequestManager::clear(target) which does the same as does RequestManager::onDestroy() but works for the concrete view. This method is present in the latest code from master and I wonder if it can be cherry picked to the 3.0 branch. This method would be very useful for unbinding custom views which use Glide.with(activity) from their RequestManager when they are untouched from window.
2. Add new lifecycle callback for onDestroyView() and handle it in different way that onDestroy() does. It should clear only targets which extend ViewTarget. It would be universal out of the box solution to avoid memory leaks of views in fragments.

[electrolobzik]

If somebody uses onDestroy() as a temporary solution he should be careful to avoid #850. See my comment in the issue for more details.

[SiddarthG]

Hi, is it ok to do this

@Override protected void onDestroy() { super.onDestroy(); new ClearDiskCache(this).execute(); }

ClearDiskCache is an AsyncTask, i'm passing 'this' from onDestroy will it be null(this)?

[TWiStErRob]

@SiddarthG this can never be null (by Java specification). You can lose the reference to whatever this is if stored in a WeakReference in the AsyncTask, and the object GC'd before doInBackground is called. Normal references (Context context;) will hold a hard reference, which means that your Activity/Fragment will be leaked (since you escaped this right before it was about to be removed from memory), hopefully just for a moment, but LeakCanary may complain. getApplicationContext() would be probably better than this for the leak. You should use the Glide API provided (maybe you already use this) to interact with its internals: https://bumptech.github.io/glide/doc/caching.html#clearing-the-disk-cache.

[rd7773]

Glide version 4.13.1

Issue: App crash in RequestManager class. IllegalStateException: Cannot unregister not yet registered manager

On checking the code in RequestManager, i see in the constructor we are adding 'this' (RequestManager) as listener to the lifecycle object (ActivityFragmentLifecycle) using fun addListener. Now in addListener fun of ActivityFragmentLifecycle, we are calling onDestroy() on this listener if lifecycle is destroyed which further calls glide.unregisterRequestManager() but as we are registering request manager only after adding the listener in RequestManager constructor, it's crashing.