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

Memory leaks when setting up Recycler/Adapter #1780

Open
anhanh11001 opened this issue May 17, 2019 · 21 comments · Fixed by #1766
Open

Memory leaks when setting up Recycler/Adapter #1780

anhanh11001 opened this issue May 17, 2019 · 21 comments · Fixed by #1766
Labels

Comments

@anhanh11001
Copy link
Contributor

Describe the bug
In most fragment, setting up RecyclerView and Adapter leads to memory leaks reported by Leak Canary

Screenshots

Additional context
#1705 related issue

Would you like to work on the issue?
Yes

@iamareebjamal
Copy link
Member

Reopening because there might be other reasons for memory leaks. Please check @anhanh11001

@anhanh11001
Copy link
Contributor Author

@liveHarshit can you also take a look at this issue as well, I'm not sure for now

@liveHarshit
Copy link
Member

@liveHarshit can you also take a look at this issue as well, I'm not sure for now

Sure 👍
This issue needs to divide into different parts. There are multiple memory leaks. We can list them manually and solve them.

@liveHarshit
Copy link
Member

@anhanh11001 @iamareebjamal What if we use data bindings for fragments instead of using lateinit rootView, I think it is causing for memory leaks.

@iamareebjamal
Copy link
Member

It's not. The listeners are causing the memory leak. If you use the binding. binding and hence, the root view will get leaked

@liveHarshit
Copy link
Member

Okay and leaking always starts with message Fragment#mFragmentManager is not null in case of recycling adapters, I don't understand what it's mean?

@iamareebjamal
Copy link
Member

iamareebjamal commented Jun 25, 2019

Doesn't matter. It means that the fragment fragmentmanager is not null and thus leaked, leaking the fragment

@iamanbansal
Copy link
Contributor

DiffCallBack could also be the cause?
I am not sure

@iamareebjamal
Copy link
Member

Just remove the listeners in onDestroyView

@iamanbansal
Copy link
Contributor

or make it static?

@iamareebjamal
Copy link
Member

Then change the title to "Add more memory leaks"

@liveHarshit
Copy link
Member

Just remove the listeners in onDestroyView

Click listeners are removed in #1766 But still, there are memory leaks with recycler adapters -

@iamareebjamal
Copy link
Member

These clearly show swipe refresh listener not being removed

@liveHarshit
Copy link
Member

@iamareebjamal
Copy link
Member

Will see what's happening

@adityastic
Copy link
Contributor

@liveHarshit I encountered a similar issue, rootview was leaking for me. Try removing the rootview, Im not sure about open-event's codebase but someone did code the fragments similar to EventsFragment.kt

@anhanh11001
Copy link
Contributor Author

@adityastic will try

@iamareebjamal
Copy link
Member

Try removing the rootview

Not a solution

@anhanh11001 anhanh11001 removed this from the Milestone 6: Summer 2019 milestone Jul 7, 2019
@iamanbansal
Copy link
Contributor

iamanbansal commented Sep 17, 2019

also set adapter to null
https://stackoverflow.com/questions/35520946/leak-canary-recyclerview-leaking-madapter

Stack Overflow
I decided it was high time I learned how to use Leak Canary to detect Leaks within my apps, and as I always do, I tried to implement it in my project to really understand how to use the tool.

@anhanh11001
Copy link
Contributor Author

anhanh11001 commented Sep 17, 2019

also set adapter to null
https://stackoverflow.com/questions/35520946/leak-canary-recyclerview-leaking-madapter

Stack Overflow**Leak canary, Recyclerview leaking mAdapter**I decided it was high time I learned how to use Leak Canary to detect Leaks within my apps, and as I always do, I tried to implement it in my project to really understand how to use the tool.

Tried, it worked. But Areeb said it is just a workaround, not a fix so it wasn't implemented.
#1766 (comment)

Stack Overflow
I decided it was high time I learned how to use Leak Canary to detect Leaks within my apps, and as I always do, I tried to implement it in my project to really understand how to use the tool.

@iamareebjamal
Copy link
Member

@anhanh11001 It actually just prevents LeakCanary to report it as memory leak. It doesn't solve the memory leak

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

Successfully merging a pull request may close this issue.

6 participants