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

setEventListener leaks fragment #48

Closed
kletzander opened this issue Feb 7, 2020 · 4 comments · Fixed by #49
Closed

setEventListener leaks fragment #48

kletzander opened this issue Feb 7, 2020 · 4 comments · Fixed by #49

Comments

@kletzander
Copy link

When calling setEventListener from fragment instance the listener does not get unregistered when the fragment is destroyed. The reason is simple - this library registers activity lifecycle observer, but the activity outlives fragments.

At minimum this causes fragment leak, in worst case scenario it causes application crash in case the event listener touches dead fragment views/data bindings.

What we would like to have added is to use Architecture Components lifecycle support, so in addition to activity there would be a LifecycleOwner passed to setEventListener where you can then observe its lifecycle and unregister the listener automatically when the owner is destroyed, be it Activity or Fragment.

For now the workaround is to manually unregister the listener in corresponding Fragment's lifecycle methods.

@yshrsmz
Copy link
Owner

yshrsmz commented Feb 7, 2020

That sounds really nice!
Would you like to send a PR?

@kletzander
Copy link
Author

kletzander commented Feb 7, 2020

Yeah, I can definitely contribute if you don't mind adding new dependency on lifecycle architecture components.

@yshrsmz
Copy link
Owner

yshrsmz commented Feb 7, 2020

AppCompat has a dependency on AndroidX Lifecycle, and there are few apps without it(I guess), so let's do it 😄

@kletzander
Copy link
Author

kletzander commented Feb 7, 2020

I have created the following PR. However I'm not sure about all the bells and whistles around bumping version numbers, publishing, etc. Can you please help me?
#49

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

Successfully merging a pull request may close this issue.

2 participants