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

fix crash when clearing glide requests #1989

Merged
merged 1 commit into from
Nov 22, 2020
Merged

Conversation

connyduck
Copy link
Collaborator

closes #1985

We cannot control when the cancel callback is called, and not cancelling would also be a bad idea, so I followed this advice in order to fix it.

Copy link
Collaborator

@charlag charlag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's standard lifecycle now yet we still have to manage this things 🙄

@connyduck connyduck merged commit 9ea2557 into develop Nov 22, 2020
@connyduck connyduck deleted the fix_glide_clear_crash branch November 22, 2020 18:03
connyduck added a commit that referenced this pull request Dec 10, 2023
…eton (#4153)

Another fix for a memory leak. This one is not as big as #4150, but
still worth fixing for memory constrained devices imo.
The `DrawerImageLoader` implementation (a global singleton) references a
member of the `MainActivity`, 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
`onFetchUserInfoSuccess` was called while the activity was being
destroyed, causing Glide to crash the app because it can't use destroyed
activities. `onFetchUserInfoSuccess` is now no longer called in this
case because it is inside a `lifecycleScope.launch` block.)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

App crashes when changing accounts
2 participants