Zhuinden / simple-stack

[ACTIVE] Simple Stack, a backstack library / navigation framework for simpler navigation and state management (for fragments, views, or whatevers).
Apache License 2.0
1.36k stars 76 forks source link

Memory leak caused by ScopeManager.untrackEventInvocationTracker #213

Closed valeriyo closed 4 years ago

valeriyo commented 4 years ago

simple-stack v2.2.1

I added LeakCanary into my app, and it caught a memory leak caused by ScopeManager.untrackEventInvocationTracker. Looking at this field, it's holding strong references to its keys, but isn't cleared out at the right time.

There are two possible solutions: 1) change untrackEventInvocationTracker to hold weak references so that GC can collect the service objects (may be non-trivial according to https://stackoverflow.com/questions/22910375/combo-of-identityhashmap-and-weakhashmap) 2) add explicit clearing of that map when it's no longer needed (required simple-stack knowledge to determine the exact place) Right now, it's cleared right before adding new objects into it.

Thanks.

Zhuinden commented 4 years ago

Backstack also holds a List<Key> and the Backstack is preserved across configuration changes, what exactly are you storing in them?

valeriyo commented 4 years ago

I store "view model" objects that should survive configuration changes. The problem with untrackEventInvocationTracker specifically is that it's holding service objects after onServiceUnregistered has been called - i.e. the service object is effectively finished, and should be GC'd.

Zhuinden commented 4 years ago

Hm, I was not expecting that map to have a significant effect. It's usually context chains and views that cause significant memory leaks.

EDIT: although if it holds services, that can hold data...yeah, actually, that's definitely an issue.

Nonetheless, the map is an instance variable to avoid creating a new map per each scope, but there is no significance to retaining the keys. I can clear them after the loop, so I'll do that. Thanks for the report.

Zhuinden commented 4 years ago

Please verify 2.2.2 (https://github.com/Zhuinden/simple-stack/commit/e1287dc956f24a36d7c5be6390fa8f770013fdda), should be fixed.

valeriyo commented 4 years ago

So far so good - thank you for the quick fix! 👍