Mapbox-gl-native: Potential crash in removeAnnotations

[mathieu-ouvrard]

Use Case:

I want to remove an annotation when the user deselects it.
This behaviour is implemented through the delegate method mapView:didDeselectAnnotation: as can be seen the 'iosap' test app on my branch.

Steps:

• add annotations (via the Debugging menu)
• select one of them
• remove all annotations (via the Debugging menu)

Crash:

When removeAnnotations: is called, it leads to a call to mapView:didDeselectAnnotation: which itself calls removeAnnotations: in the same callstack which provokes a KVO crash on removeObserver.

Expected:

Calling removeAnnotation(s) multiple times on the same annotation(s) must remove the annotation only once.

[mathieu-ouvrard]

Issue can be seen on branch 13594-crash_removeAnnotation 62417312f7bd645f2685a81c9d877d79eb218f7c

[mathieu-ouvrard]

I can see 2 ways to fix this issue :

• create a safe-KVO wrapper : to ensure add/remove operations are done only once
• add a check in removeAnnotations:

[mathieu-ouvrard]

The second option is implemented in 0e22221708c6d91e721b1b5ac6572724f9b2a56e

[julianrex]

Thanks for the report @floydaddict!

In the meantime can you bypass this issue by removing the annotation from a dispatch_async called from the delegate method?

[mathieu-ouvrard]

@julianrex yes, that will work for now, thanks

[julianrex]

@floydaddict do you have a sample project that demonstrates this crash? Adding the code from https://github.com/mapbox/mapbox-gl-native/commit/62417312f7bd645f2685a81c9d877d79eb218f7c and creating a default annotation (long press) doesn't crash for me.

Are you KVO'ing the annotation independently of the map view?

[mathieu-ouvrard]

@julianrex sorry the step to reproduce were not very clear, it took me a while to remember how to trigger the crash, I'm gonna update the issue with more info.

[mathieu-ouvrard]

The crash is no longer happening with the latest version of removeAnnotations of MGLMapView from master.

@julianrex thanks for your time.

[julianrex]

@floydaddict thanks for checking - please feel free to reopen should you see it again.