prevent MainActivity from leaking through the DrawerImageLoader singleton #4153
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Tak
approved these changes
Dec 8, 2023
|
I did some more testing on another device, still can't reproduce. Merging so nightly testers get to use it. |
connyduck
added a commit
that referenced
this pull request
Dec 24, 2023
Turns out that the crash I thought will not occur with #4153 did occur again. 😒 But this time I managed to reproduce it (faking a slow network for that one request only and then quickly switching activities to get it destroyed) so I'm sure it is fixed, and I did a check for possible side effects but it seems Glide is clever enough to cancel all requests by itself. <details> <summary>stacktrace</summary> ``` Exception java.lang.IllegalArgumentException: You cannot start a load for a destroyed activity at com.bumptech.glide.manager.RequestManagerRetriever.assertNotDestroyed (RequestManagerRetriever.java:236) at com.bumptech.glide.manager.RequestManagerRetriever.get (RequestManagerRetriever.java:110) at com.bumptech.glide.manager.RequestManagerRetriever.get (RequestManagerRetriever.java:176) at com.bumptech.glide.Glide.with (Glide.java:634) at com.keylesspalace.tusky.MainActivity$setupDrawer$2.cancel (MainActivity.kt:573) at com.mikepenz.materialdrawer.util.DrawerImageLoader.cancelImage (DrawerImageLoader.java:56) at com.mikepenz.materialdrawer.model.BaseDescribeableDrawerItem.unbindView (BaseDescribeableDrawerItem.kt:95) at com.mikepenz.materialdrawer.model.BaseDescribeableDrawerItem.unbindView (BaseDescribeableDrawerItem.kt:20) at com.mikepenz.fastadapter.listeners.OnBindViewHolderListenerImpl.unBindViewHolder (OnBindViewHolderListenerImpl.java:36) at com.mikepenz.fastadapter.FastAdapter.onViewRecycled (FastAdapter.kt:410) at androidx.recyclerview.widget.RecyclerView$Recycler.dispatchViewRecycled (RecyclerView.java:7346) at androidx.recyclerview.widget.RecyclerView$Recycler.addViewHolderToRecycledViewPool (RecyclerView.java:7108) at androidx.recyclerview.widget.RecyclerView$Recycler.recycleViewHolderInternal (RecyclerView.java:7059) at androidx.recyclerview.widget.RecyclerView.removeAnimatingView (RecyclerView.java:1556) at androidx.recyclerview.widget.RecyclerView$ItemAnimatorRestoreListener.onAnimationFinished (RecyclerView.java:13564) at androidx.recyclerview.widget.RecyclerView$ItemAnimator.dispatchAnimationFinished (RecyclerView.java:14066) at androidx.recyclerview.widget.SimpleItemAnimator.dispatchChangeFinished (SimpleItemAnimator.java:328) at androidx.recyclerview.widget.DefaultItemAnimator.endChangeAnimationIfNecessary (DefaultItemAnimator.java:437) at androidx.recyclerview.widget.DefaultItemAnimator.endChangeAnimationIfNecessary (DefaultItemAnimator.java:418) at androidx.recyclerview.widget.DefaultItemAnimator.endAnimations (DefaultItemAnimator.java:588) at androidx.recyclerview.widget.RecyclerView.onDetachedFromWindow (RecyclerView.java:3383) at android.view.View.dispatchDetachedFromWindow (View.java:20898) at android.view.ViewGroup.dispatchDetachedFromWindow (ViewGroup.java:3956) at android.view.ViewGroup.dispatchDetachedFromWindow (ViewGroup.java:3948) at android.view.ViewGroup.dispatchDetachedFromWindow (ViewGroup.java:3948) at android.view.ViewGroup.dispatchDetachedFromWindow (ViewGroup.java:3948) at android.view.ViewGroup.dispatchDetachedFromWindow (ViewGroup.java:3948) at android.view.ViewGroup.dispatchDetachedFromWindow (ViewGroup.java:3948) at android.view.ViewGroup.dispatchDetachedFromWindow (ViewGroup.java:3948) at android.view.ViewGroup.dispatchDetachedFromWindow (ViewGroup.java:3948) at android.view.ViewRootImpl.dispatchDetachedFromWindow (ViewRootImpl.java:5114) at android.view.ViewRootImpl.doDie (ViewRootImpl.java:8309) at android.view.ViewRootImpl.die (ViewRootImpl.java:8286) at android.view.WindowManagerGlobal.removeViewLocked (WindowManagerGlobal.java:538) at android.view.WindowManagerGlobal.removeView (WindowManagerGlobal.java:479) at android.view.WindowManagerImpl.removeViewImmediate (WindowManagerImpl.java:162) at android.app.ActivityThread.handleDestroyActivity (ActivityThread.java:5564) at android.app.ActivityThread.handleRelaunchActivityInner (ActivityThread.java:5842) at android.app.ActivityThread.handleRelaunchActivity (ActivityThread.java:5758) at android.app.servertransaction.ActivityRelaunchItem.execute (ActivityRelaunchItem.java:71) at android.app.servertransaction.ActivityTransactionItem.execute (ActivityTransactionItem.java:45) at android.app.servertransaction.TransactionExecutor.executeCallbacks (TransactionExecutor.java:135) at android.app.servertransaction.TransactionExecutor.execute (TransactionExecutor.java:95) at android.app.ActivityThread$H.handleMessage (ActivityThread.java:2293) at android.os.Handler.dispatchMessage (Handler.java:106) at android.os.Looper.loopOnce (Looper.java:226) at android.os.Looper.loop (Looper.java:329) at android.app.ActivityThread.main (ActivityThread.java:8058) at java.lang.reflect.Method.invoke at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run (RuntimeInit.java:548) at com.android.internal.os.ZygoteInit.main (ZygoteInit.java:1026) ``` </details>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Another fix for a memory leak. This one is not as big as #4150, but still worth fixing for memory constrained devices imo.
The
DrawerImageLoaderimplementation (a global singleton) references a member of theMainActivity, causing the whole activity to leak.This weird construct was introduced in #1989 to fix a crash, but I think since we migrated to coroutines it is no longer necessary because all calls get correctly cancelled. I tried reproducing the crash but could not, so I'm pretty sure it is fine. I would appreciate it if someone else could try it as well though.
(The crash could be reproduced on slow internet, when
onFetchUserInfoSuccesswas called while the activity was being destroyed, causing Glide to crash the app because it can't use destroyed activities.onFetchUserInfoSuccessis now no longer called in this case because it is inside alifecycleScope.launchblock.)