fossasia / open-event-attendee-android

Open Event Attendee Android General App https://github.com/fossasia/open-event-android/blob/apk/open-event-dev-app-playStore-debug.apk
Apache License 2.0
1.95k stars 551 forks source link

Memory leaks when setting up Recycler/Adapter #1780

Open anhanh11001 opened 5 years ago

anhanh11001 commented 5 years ago

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 commented 5 years ago

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

anhanh11001 commented 5 years ago

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

liveHarshit commented 5 years ago

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

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

liveHarshit commented 5 years ago

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

iamareebjamal commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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

iamanbansal commented 5 years ago

DiffCallBack could also be the cause? I am not sure

iamareebjamal commented 5 years ago

Just remove the listeners in onDestroyView

iamanbansal commented 5 years ago

or make it static?

iamareebjamal commented 5 years ago

Then change the title to "Add more memory leaks"

liveHarshit commented 5 years ago

Just remove the listeners in onDestroyView

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

iamareebjamal commented 5 years ago

These clearly show swipe refresh listener not being removed

liveHarshit commented 5 years ago

These clearly show swipe refresh listener not being removed

But it is set to null - https://github.com/fossasia/open-event-android/blob/91eda72bab0ed86f564ed26d259b7acb1eff7c7a/app/src/main/java/org/fossasia/openevent/general/event/EventsFragment.kt#L209

iamareebjamal commented 5 years ago

Will see what's happening

adityastic commented 5 years ago

@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 commented 5 years ago

@adityastic will try

iamareebjamal commented 5 years ago

Try removing the rootview

Not a solution

iamanbansal commented 5 years ago

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.
anhanh11001 commented 5 years ago

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

Stack OverflowLeak canary, Recyclerview leaking mAdapterI 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. https://github.com/fossasia/open-event-attendee-android/pull/1766#issuecomment-492699626

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.
iamareebjamal commented 5 years ago

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