mapbox / mapbox-maps-android

Interactive, thoroughly customizable maps in native Android powered by vector tiles and OpenGL.
https://www.mapbox.com/mobile-maps-sdk
Other
475 stars 132 forks source link

MapView Memory leak #2184

Open guitabalipa opened 1 year ago

guitabalipa commented 1 year ago

Environment

Observed behavior and steps to reproduce

A memory leak started to occur a while ago, it doesn't seem be related with any changes in our implementation but something internal in the SDK. The leak happens every time we enter a screen that has the map view. Also, we were only able to reproduce it when using Don't keep activities settings

┬───
│ GC Root: Global variable in native code
│
├─ com.mapbox.maps.NativeObserver instance
│    Leaking: UNKNOWN
│    Retaining 114.0 kB in 2977 objects
│    ↓ NativeObserver.onMapLoadErrorListeners
│                     ~~~~~~~~~~~~~~~~~~~~~~~
├─ java.util.concurrent.CopyOnWriteArraySet instance
│    Leaking: UNKNOWN
│    Retaining 40 B in 4 objects
│    ↓ CopyOnWriteArraySet.al
│                          ~~
├─ java.util.concurrent.CopyOnWriteArrayList instance
│    Leaking: UNKNOWN
│    Retaining 28 B in 3 objects
│    ↓ CopyOnWriteArrayList[0]
│                          ~~~
├─ com.mapbox.maps.StyleObserver instance
│    Leaking: UNKNOWN
│    Retaining 113.4 kB in 2920 objects
│    ↓ StyleObserver.styleLoadedListener
│                    ~~~~~~~~~~~~~~~~~~~
├─ com.mapbox.maps.MapboxMap$$ExternalSyntheticLambda1 instance
│    Leaking: UNKNOWN
│    Retaining 113.2 kB in 2914 objects
│    ↓ MapboxMap$$ExternalSyntheticLambda1.f$0
│                                          ~~~
├─ com.mapbox.maps.MapboxMap instance
│    Leaking: UNKNOWN
│    Retaining 113.2 kB in 2913 objects
│    ↓ MapboxMap.gesturesPlugin
│                ~~~~~~~~~~~~~~
├─ com.mapbox.maps.plugin.gestures.GesturesPluginImpl instance
│    Leaking: UNKNOWN
│    Retaining 3.5 kB in 118 objects
│    context instance of com.example.MainActivity with mDestroyed = false
│    ↓ GesturesPluginImpl.mapPluginProviderDelegate
│                         ~~~~~~~~~~~~~~~~~~~~~~~~~
├─ com.mapbox.maps.MapController instance
│    Leaking: UNKNOWN
│    Retaining 105.1 kB in 2598 objects
│    ↓ MapController.pluginRegistry
│                    ~~~~~~~~~~~~~~
├─ com.mapbox.maps.plugin.MapPluginRegistry instance
│    Leaking: UNKNOWN
│    Retaining 103.5 kB in 2556 objects
│    ↓ MapPluginRegistry.plugins
│                        ~~~~~~~
├─ java.util.LinkedHashMap instance
│    Leaking: UNKNOWN
│    Retaining 608 B in 18 objects
│    ↓ LinkedHashMap["MAPBOX_ATTRIBUTION_PLUGIN_ID"]
│                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
├─ com.mapbox.maps.plugin.attribution.AttributionViewPlugin instance
│    Leaking: UNKNOWN
│    Retaining 94 B in 3 objects
│    ↓ AttributionViewPlugin.attributionView
│                            ~~~~~~~~~~~~~~~
├─ com.mapbox.maps.plugin.attribution.AttributionViewImpl instance
│    Leaking: UNKNOWN
│    Retaining 2.7 kB in 58 objects
│    View not part of a window view hierarchy
│    View.mAttachInfo is null (view detached)
│    View.mWindowAttachCount = 0
│    mContext instance of com.example.MainActivity with mDestroyed = false
│    ↓ View.mParent
│           ~~~~~~~
├─ com.mapbox.maps.MapView instance
│    Leaking: UNKNOWN
│    Retaining 90.6 kB in 2261 objects
│    View not part of a window view hierarchy
│    View.mAttachInfo is null (view detached)
│    View.mID = R.id.map_view
│    View.mWindowAttachCount = 0
│    mContext instance of com.example.MainActivity with mDestroyed = false
│    ↓ View.mParent
│           ~~~~~~~
╰→ androidx.constraintlayout.widget.ConstraintLayout instance
     Leaking: YES (ObjectWatcher was watching this because com.example.
​     MapboxFragment received Fragment#onDestroyView() callback (references to its views should be cleared to prevent
​     leaks))
​     Retaining 84.3 kB in 2155 objects
​     key =
​     watchDurationMillis = 5808
​     retainedDurationMillis = 808
​     View not part of a window view hierarchy
​     View.mAttachInfo is null (view detached)
​     View.mID = R.id.map_root_container
​     View.mWindowAttachCount = 0
​     mContext instance of com.example.MainActivity with mDestroyed = false

Expected behavior

No memory leak.

Notes / preliminary analysis

Additional links and references

dudeuter commented 1 year ago

@guitabalipa it looks like your application is holding onto a reference to the MapView.

com.example.MainActivity Do you have this reproducible in a stand-alone application? Also what versions did this start happening in? What version can you roll back to that stops the leak from occurring?

guitabalipa commented 1 year ago

@dudeuter No, Ive only tried to reproduce in our project, I just changed the package to com.example for anonymity.

I tried to test with all the versions that we used in the past year: 10.16.0 -> has the leak 10.10.1 -> has the leak 10.8.1 -> has the leak 10.7.0 -> Do not have the leak

Between those versions we didn't change much the implementation code.

Also, the 10.7.0 version is the only one that uses the 6.7.0 Mapbox sdk services version, all the others use 6.8.0. Dont know if this can be related, only add it for your information.

dudeuter commented 1 year ago

@guitabalipa Taking another look at this. Is it possible for you to take a capture of your Java heap to visualize in perfetto ui? Or maybe create a stand-alone example that can reproduce this leak?

Right now from what you've shared, it seems like there's likely a leak in the implementation code, and I haven't heard of such a leak from any other users. Not to dismiss this issue, but I don't have any direct clues to root cause.

guitabalipa commented 1 year ago

@dudeuter Is there any secure channel that I could send the java heap file for you? I don't want to send it here since it could contain some company internal information.

dudeuter commented 1 year ago

@guitabalipa Certainly, and apologies for not responding sooner; I've had a lot of actions on my end the past few days. Can you create a ticket on our Support Form and mention my handle in the body of your request? Our Support Team will assign the ticket to me. You should be able to upload attachments there as well.

guitabalipa commented 1 year ago

Thanks @dudeuter, just sent the form, let me know if you need anything more.

vladteleport commented 7 months ago

This memory leak can still be reproduced in version 11.2.2

dudeuter commented 7 months ago

Hi @vladteleport,

This is likely an issue in your implementation. I'm unable to reproduce this internally. I would recommend using the Android Tools to profile your application. https://developer.android.com/studio/profile/memory-profiler

Asgarde commented 5 months ago

This memory leak can still be reproduced in version 11.2.2

There is a memory leak in versions 11.2 and above. By using MVVM and Hilt, I shouldn't have a problem with persistent Mapbox references. However, when I return to the map 3 or 4 times, the app crashes. I am forced to destroy the map each time I exit, by calling mapview.onDestroy() to ensure that instances are not duplicated. This is the only solution I have found.

marat-tlepberghenov commented 4 days ago

@Asgarde Thanks for sharing your idea,

Also mapview.onDestroy() works for me.

flasher297 commented 2 days ago

As mentioned in previous comments, you have to call mapview.onDestroy()

flasher297 commented 9 hours ago

After internal discussion, we decided to reopen the issue and investigate why in certain cases you need to call mapview.onDestroy()manually in SDK v 11.2.2+. So I've already opened an internal ticket. I would appreciate it if anybody could provide an example that reproduces the issue.