search

bgogetap / StickyHeaders

Easily add Sticky Headers to your RecyclerView
Apache License 2.0
521 stars 88 forks source link

[Memory Leak] StickyHeaderPositioner has uncleared GlobalLayoutListener #72

Closed tikimcfee closed 5 years ago

tikimcfee commented 6 years ago

Hey there!

I'll start off with much fanfare and thanks for creating this lib. Tons of sticky libraries have come and gone, and to see this one be as successful and functional as it is already is awesome. Seriously, great stuff.

Anyway. I come with a leak report. It seems in StickyHeaderPositioner, the constructor has a new OnGlobalLayoutListener watching for visibility changes. Problem with that guy is that it's never unattached.

This doesn't seem too terrible; there's already an extended onAttachedToWindow in the StickyLayoutManager. Might be able to extend the onDetached counterpart and 'clean up' the Positioner's layout listener. If you're low on time what with the recent 0.5.0 release, I might be able to put together a PR if you've got a preferred way of handling things.

Thanks again for your time and the utility - have a good weekend!

bgogetap commented 5 years ago

Super late response--I'm sorry.

This didn't really cross my mind as I don't have a use case of removing a RecyclerView from a ViewGroup while continuing to use the ViewGroup.

If the RecyclerView has the same lifecycle as it's view tree, this won't cause a leak (and even then, it will be just a temporary leak of the observer itself). The Context itself will never leak. Just wanted to clarify this for future readers (and give someone a chance to correct me if I'm wrong).

I'll be approving #90 after some minor changes to fix this.

bgogetap commented 5 years ago

Fixed by #90