Skip to content
This repository has been archived by the owner on Aug 8, 2023. It is now read-only.

Crash on open close different MapView #9096

Closed
evgzor opened this issue May 24, 2017 · 15 comments
Closed

Crash on open close different MapView #9096

evgzor opened this issue May 24, 2017 · 15 comments
Labels
annotations Annotations on iOS and macOS or markers on Android beta blocker Blocks the next beta release crash iOS Mapbox Maps SDK for iOS needs information
Milestone

Comments

@evgzor
Copy link

evgzor commented May 24, 2017

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:
screenshot 2017-05-24 09 06 47
and:
screenshot 2017-05-24 10 48 48

and crash log: crash.crash.zip

In previous version 3.5.4 - no crash observed with same scenario

@tobrun tobrun added the iOS Mapbox Maps SDK for iOS label May 24, 2017
@fabian-guerra fabian-guerra added annotations Annotations on iOS and macOS or markers on Android crash labels May 24, 2017
@fabian-guerra fabian-guerra added this to the ios-v3.6.0 milestone May 24, 2017
@boundsj
Copy link
Contributor

boundsj commented May 24, 2017

@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
Copy link
Author

evgzor commented May 24, 2017

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
Copy link
Contributor

jmkiley commented May 24, 2017

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

@boundsj boundsj added release blocker Blocks the next final release beta blocker Blocks the next beta release and removed release blocker Blocks the next final release labels May 25, 2017
@incanus
Copy link
Contributor

incanus commented May 25, 2017

@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
Copy link
Contributor

incanus commented May 25, 2017

@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
Copy link
Contributor

incanus commented May 25, 2017

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
Copy link
Contributor

jmkiley commented May 25, 2017

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: <NSKeyValueObservationInfo 0x600000430880> (
<NSKeyValueObservance 0x600000254d30: Observer: 0x7fb107857c00, Key path: bounds, Options: <New: NO, Old: NO, Prior: NO> Context: 0x10879ac90, Property: 0x600000254d60>

@incanus
Copy link
Contributor

incanus commented May 25, 2017

a clean MGLMapView in iosapp

537355e

@frederoni
Copy link
Contributor

frederoni commented May 25, 2017

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
Copy link
Contributor

boundsj commented May 30, 2017

This was fixed by #9109 on the release-ios-v3.6.0-android-v5.1.0 branch.

@boundsj boundsj closed this as completed May 30, 2017
@evgzor
Copy link
Author

evgzor commented May 31, 2017

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

@frederoni
Copy link
Contributor

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

@evgzor
Copy link
Author

evgzor commented May 31, 2017

when 3.6.0 would be released for checking fix?

@frederoni
Copy link
Contributor

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

@boundsj
Copy link
Contributor

boundsj commented May 31, 2017

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
annotations Annotations on iOS and macOS or markers on Android beta blocker Blocks the next beta release crash iOS Mapbox Maps SDK for iOS needs information
Projects
None yet
Development

No branches or pull requests

8 participants