Mapbox-gl-native: Crash on open close different MapView

[evgzor]

iOS 10.3.2
v3.6.0-beta.1

Steps to trigger behavior

1. Open MGLMapview with multiple annotations
2. Close MapView with multiple annotations
3. Open another MGLMapview with multiple annotations

Expected behavior

No crash observed

Actual behavior

Crash observed like:

and:

and crash log: crash.crash.zip

In previous version 3.5.4 - no crash observed with same scenario

[boundsj]

@evgzor from what I can tell, the crash report you attached does not implicate the Mapbox SDK. Can you please provide a sample project or code in a gist that would allow us to reproduce this?

[evgzor]

Dear @boundsj this behavior is easily reproducible on yours ios-sdk examples with multiple annotations. If u want I can attach video. Just open select all annotations
and press back button. bug.zip -- just make your life easy: commit ec1a50ca2b7ed35836eefb789d7fbd8ae92fbd20

[jmkiley]

Hi @evgzor,

I was able to repro this issue in ios-sdk-examples using v3.6.0-beta.1. The issue seems to have been introduced in that version, as I am not seeing it with previous versions.

Looks like it may be related to #7716 Will look more into this!
cc @frederoni @friedbunny

[incanus]

@jmkiley and I found that the unregistering of bounds observation happening in -[MGLMapView dealloc] as part of #7716 is too late in the lifecycle—as is -removeFromSuperview. Both occasions are too late in the breakdown cycle to have properly unregistered the observation.

[incanus]

@frederoni It also seems to be the case that if we (the MGLMapView) were moved between view controllers, we don't pay attention to that view controller's lifecycle to know to unregister observing its layout guides (say, during a UIViewAnimationTransition between view controllers). So we'd be in a new view controller but still paying attention to the layout guides from the first one.

[incanus]

properly unregistered the observation

To be clear, the crash is an unhandled exception related to the view controller layout guides going away before we unregister observing the layout guides' bounds.

[jmkiley]

Noting that this crash seems to happen regardless of whether or not there are annotations or a nav bar. @incanus was able to reproduce with a clean MGLMapView in iosapp.

My next steps are to look more into the UIView lifecycle to identify where the crash is happening.

Including the message from ios-sdk-examples for searchability:

2017-05-25 10:35:34.033 Examples[26372:856825] * Terminating app due to uncaught exception 'NSInternalInconsistencyException', reason: 'An instance 0x6000001b5d20 of class _UILayoutSpacer was deallocated while key value observers were still registered with it. Current observation info: (
Context: 0x10879ac90, Property: 0x600000254d60>

[incanus]

a clean MGLMapView in iosapp

https://github.com/mapbox/mapbox-gl-native/commit/537355e16da43f38c3678b72d4bc265cf2883753

[frederoni]

To be clear, the crash is an unhandled exception related to the view controller layout guides going away before we unregister observing the layout guides' bounds.

This seems to be correct. _UILayoutSpacer is deallocated before MGLMapView is. I think we can solve this by overriding -[UIView willMoveToWindow:] and remove observers when window is nil.

we don't pay attention to that view controller's lifecycle to know to unregister observing its layout guides (say, during a UIViewAnimationTransition between view controllers).

Good point. This should work with the willMoveToWindow fix mentioned aboved.

[boundsj]

This was fixed by https://github.com/mapbox/mapbox-gl-native/pull/9109 on the release-ios-v3.6.0-android-v5.1.0 branch.

[evgzor]

It's ios issue. How it's can be close as Android issue?

[frederoni]

@evgzor if you are referring to the branch name, iOS and Android share a release branch. However, this issue only affects iOS.

[evgzor]

when 3.6.0 would be released for checking fix?

[frederoni]

3.6.0-beta.2 is just around the corner. Perhaps at the end of this week.

[boundsj]

@evgzor 3.6.0-beta.2 should be published a few hours from now.