Mapbox-gl-native: ANR in onDestroy() when using Mapbox in a Conductor controller

[almeric]

Platform: Android
Mapbox SDK version: 5.2.0-beta.1

Steps to trigger behavior

I'm trying to use MapBox in a Conductor project. With the 5.2 beta version I get a black screen and a 'app is not responding' notification after onDestroy() is called on the mapview. This works without any problems on the 5.1.4 version of the MapBox SDK.

Attached is a sample project which demonstrates this behaviour, I had to juggle a bit with lifecycle methods as a controller has fewer lifecycle methods than a Fragment/Activity.

To Reproduce: Build the app, press go button, go back

Also attached is the trace file.
traces.txt
ConductorMapBox.zip

Expected behavior

No ANR after calling onDestroy()

Actual behavior

ANR after calling onDestroy()

[tobrun]

Hey @almeric, thank you for adding the example project! this makes getting to the bottom of the issue more easy! I looked into the example project and noticed that onDestroy of the MapView is correctly called but the Activity itself was never destroyed:

override fun onBackPressed() {
        Log.e("ContentValues", "OnBackPressed")
        if(!router.handleBack()) {
            Log.e("ContentValues", "super.OnBackPressed")
            super.onBackPressed()
        }
    }

In this case I'm only seeing ""OnBackPressed"" and no additional "super.OnBackPressed" in the logcat. This means that the activity will not be destroyed though you are trying to destroy the MapView. Making sure that the state of both are equal will result in a correct behaviour. OnDestroy on MapView should only be called when the host (either fragment or Activity) is going to be destroyed in the example project this doesn't seem to be the case.

[tobrun]

Disabling the router.handleBack call and hooking into super.onBackPressed does work as designed. I'm going to close this as this doesn't seem to be an issue from our end. (though it could be that we are more strict about when onDestroy is called due to moving to a glsurfaceview implementation). Happy to hear any follow up information or discussion. Closing as non-actionable for now.

[almeric]

Hi @tobrun, thanks for looking in to this issue. Makes sense. The problem is that the app I'm working on has a single activity into which all controllers are loaded. The controllers being lightweight fragments in this case. So the activity will only get cleaned up if the user exits the app.

From looking at the memory profiler I can see that memory usage keeps increasing if I keep opening and closing the MapController without the onDestroy call. This isn't an issue in the older version of MapBox where I can call onDestroy without problems, and the mapview gets cleaned up.

Any ideas on this? Or is this more an issue for the Conductor project?

[kmityakov]

@almeric
After hours of investigation I came to the following solution:

override fun onDestroyView(view: View) {
        super.onDestroyView(view)
        mapView.visibility = View.GONE
        removeFromSuperview(mapView)
        activity?.findViewById<ViewGroup>(R.id.container)?.addView(mapView)
        mapView.onDestroy()
        removeFromSuperview(mapView)
    }

fun removeFromSuperview(view: View) {
        val parent = view.parent
        if (parent != null && parent is ViewGroup) {
            parent.removeView(view)
        }
}

Explanation:
It turns out that when you call MapView.onDestroy() it's expected that MapView is attached to the window (i.e., added to root view). However, when conductor calls onDestroyView() the controller's view is already removed from the router's container, that's why during MapView.onDestroy() it freezes somewhere on the native layer. To overcome this issue, you can add MapView to root container, call onDestroy, and then remove MapView.
The example is written in Kotlin, but I hope the point is clear.

Hope this will help.

[almeric]

@kmityakov
That's an interesting solution, thanks!

We've reverted to Google Maps due to the ANR, but I'll keep this in mind. Thanks again!

[tikimcfee]

@kmityakov Thanks for doing the research on this!! We've been experiencing these issues a lot as well, and hadn't had time to dig into it. I'm going to try this and see how it works for us.. stay tuned!

EDIT:
Ho. Lee. Smokes. So dropping that in to our application has fixed all of the timing crashes we've seen on MapView teardown. What a fascinating combination of issues between Conductor and MapBox (really, Android Surface/TextureViews).

With this is mind, is there anything that MBGL can do as a minor patch that can fix this internally? I totally understand the _view being swept out from underneath you_ is a bit of massive headache, but there are View attach listeners to grab a hold of; maybe you could use one of those, perhaps optionally (like a setShouldObserveRootViewLifecycle or something equally verbose ;) ) Otherwise, by using this patch there ends up being a nice, chunky second-or-two frame drop while the map is removed, added, torn down, and then removed again in the view's detach flow.

EDIT 2:
Also, sorry to revive a closed issue. Let me know if you'd like me to reopen a new one as a bump. I'm just adding that there are some existing tickets looking to do something along the lines of what I mentioned above - the one that grabbed my attention first was #11685. Any idea if that's coming along?

[tobrun]

@kmityakov @tikimcfee, thanks for chiming in, #11685 is aimed to fix this issue so no need to create a new one. #11685 is on the backlog but we haven't had time to prioritise it. As long as a developer calls MapView#onDestroy before the view is removed, the issue should not arise. Thanks again all for reaching out!

[Marchuck]

Hello there, thanks for your insights,
not sure this helps someone, but I encountered a few rarely happening ANR during onDestroy when dealing with mapbox map inside fragment.

I had dozens of annotations on the map(LineManagers/SymbolManagers) so I needed to call onDestroy() on each of AnnotationManager before exiting the fragment, to prevent leaks.

in my case implementation was broken, because I was calling onDestroy manually,
when I was clicking back button. SIGSEGV native crashes appeared. 😰

simply, I just intriduced tiny monitor of lifecycle (onResume / onPause/onStop) in order to always fit to the _natural_ map lifecycle: RESUMED->PAUSED->STOPPED->DESTROYED:

fun customOnDestroy() {
 if (mapResumed()) {
     mapView?.onPause()
     mapView?.onStop()
  }
  if (mapPaused()) {
      mapView?.onStop()
  }
  lineManager?.onDestroy()
  markerSymbolManager?.onDestroy()
  someOtherSymbolManager?.onDestroy()
  mapView?.onDestroy()
}