search

cyclestreets / android

The Android app brings CycleStreets routing and turn-by-turn live navigation to your phone.
https://www.cyclestreets.net/mobile/android/
GNU General Public License v3.0
214 stars 217 forks source link

Memory leak in Route class #477

Open HilaryN opened 3 years ago

HilaryN commented 3 years ago

Android Studio is giving a warning in the Route.kt class: "Do not place Android context classes in static fields (static reference to 'Route' which has field context_ pointing to Context); this is a memory leak."

oliverlockwood commented 3 years ago

I think this issue is quite widespread, and may be quite involved to refactor away from. Given we've not (to my knowledge) observed any issues as a result of this, I recognise the issue but wouldn't say we should necessarily give it a high priority.

HilaryN commented 3 years ago

Agreed, yes.

HilaryN commented 3 years ago

LiveRide freezes on my Fairphone 2 (though fine on my FP3) and I've tracked it down to this. I'm working on a fix - it seems to be possible to pass the context as a parameter in the functions which need it. It only freezes when moving (cycling / walking / other transport!). It also freezes when in Journey Planner and moving. I'm not quite sure what that has to do with the Route class, but if I take out context_ it doesn't seem to freeze.

HilaryN commented 2 years ago

Now I'm not so sure that's the problem. It doesn't freeze on commit 5652817 / 1st May 2020 / "Audit CycleStreets.net/cyclestreets.org links", but freezes on commits after that. If I take 5652817 and convert the Route class to Kotlin without doing any other change except minimal ones for it to work, then it freezes. If I then change context to be passed as a parameter rather than stored in the static var it still freezes :-( If I then remove LocationOverlay it doesn't freeze. (A bit hard to navigate without the LocationOverlay, but just about possible! :-) )