Fix crash when quitting while a map view has annotations

platform/ios/CHANGELOG.md

@@ -30,6 +30,7 @@ Mapbox welcomes participation and contributions from everyone. Please read [CONT
 ### Other changes
 
 * Added the `MGLMapView.minimumPitch` and `MGLMapView.maximumPitch` properties to further limit how much the user or your code can tilt the map. ([#208](https://github.com/mapbox/mapbox-gl-native-ios/pull/208))
+* Fixed a crash while quitting the application after adding an annotation to `MGLMapView`. ([#252](https://github.com/mapbox/mapbox-gl-native-ios/pull/252))
 * Improved performance when continuously animating a tilted map. ([#16287](https://github.com/mapbox/mapbox-gl-native/pull/16287))
 * Fixed a memory leak when zooming with any options enabled in the `MGLMapView.debugMask` property. ([#15179](https://github.com/mapbox/mapbox-gl-native/issues/15179))
 

platform/ios/src/MGLMapView.mm

@@ -280,13 +280,11 @@ @interface MGLMapView () <UIGestureRecognizerDelegate,
 @property (nonatomic, assign) UIEdgeInsets safeMapViewContentInsets;
 @property (nonatomic, strong) NSNumber *automaticallyAdjustContentInsetHolder;
 
-- (mbgl::Map &)mbglMap;
-
 @end
 
 @implementation MGLMapView
 {
-    mbgl::Map *_mbglMap;
+    std::unique_ptr<mbgl::Map> _mbglMap;
     std::unique_ptr<MGLMapViewImpl> _mbglView;
     std::unique_ptr<MGLRenderFrontend> _rendererFrontend;
 
@@ -499,7 +497,7 @@ - (void)commonInit
                    .withAssetPath([NSBundle mainBundle].resourceURL.path.UTF8String);
 
     NSAssert(!_mbglMap, @"_mbglMap should be NULL");
-    _mbglMap = new mbgl::Map(*_rendererFrontend, *_mbglView, mapOptions, resourceOptions);
+    _mbglMap = std::make_unique<mbgl::Map>(*_rendererFrontend, *_mbglView, mapOptions, resourceOptions);
 
     // start paused if in IB
     if (background) {
@@ -716,8 +714,7 @@ - (void)destroyCoreObjects {
 
     // Tear down C++ objects, insuring worker threads correctly terminate.
     // Because of how _mbglMap is constructed, we need to destroy it first.
-    delete _mbglMap;
-    _mbglMap = nullptr;
+    _mbglMap.reset();
 
     _mbglView.reset();
 
@@ -2807,12 +2804,12 @@ - (void)setShowsScale:(BOOL)showsScale
 
 - (void)setPrefetchesTiles:(BOOL)prefetchesTiles
 {
-    _mbglMap->setPrefetchZoomDelta(prefetchesTiles ? mbgl::util::DEFAULT_PREFETCH_ZOOM_DELTA : 0);
+    self.mbglMap.setPrefetchZoomDelta(prefetchesTiles ? mbgl::util::DEFAULT_PREFETCH_ZOOM_DELTA : 0);
 }
 
 - (BOOL)prefetchesTiles
 {
-    return _mbglMap->getPrefetchZoomDelta() > 0 ? YES : NO;
+    return self.mbglMap.getPrefetchZoomDelta() > 0 ? YES : NO;
 }
 
 #pragma mark - Accessibility -
@@ -3527,24 +3524,24 @@ - (double)maximumZoomLevel
 
 - (CGFloat)minimumPitch
 {
-    return *_mbglMap->getBounds().minPitch;
+    return *self.mbglMap.getBounds().minPitch;
 }
 
 - (void)setMinimumPitch:(CGFloat)minimumPitch
 {
     MGLLogDebug(@"Setting minimumPitch: %f", minimumPitch);
-    _mbglMap->setBounds(mbgl::BoundOptions().withMinPitch(minimumPitch));
+    self.mbglMap.setBounds(mbgl::BoundOptions().withMinPitch(minimumPitch));
 }
 
 - (CGFloat)maximumPitch
 {
-    return *_mbglMap->getBounds().maxPitch;
+    return *self.mbglMap.getBounds().maxPitch;
 }
 
 - (void)setMaximumPitch:(CGFloat)maximumPitch
 {
     MGLLogDebug(@"Setting maximumPitch: %f", maximumPitch);
-    _mbglMap->setBounds(mbgl::BoundOptions().withMaxPitch(maximumPitch));
+    self.mbglMap.setBounds(mbgl::BoundOptions().withMaxPitch(maximumPitch));
 }
 
 - (MGLCoordinateBounds)visibleCoordinateBounds
@@ -4615,7 +4612,12 @@ - (void)removeAnnotations:(NSArray<id <MGLAnnotation>> *)annotations
         }
 
         _isChangingAnnotationLayers = YES;
-        self.mbglMap.removeAnnotation(annotationTag);
+        // If the underlying map is gone, there’s nothing to remove, but still
+        // continue to unregister KVO and other annotation resources.
+        if (_mbglMap)
+        {
+            self.mbglMap.removeAnnotation(annotationTag);
+        }
     }
 
     [self updatePresentsWithTransaction];
@@ -6476,7 +6478,7 @@ - (void)didFailToLoadImage:(NSString *)imageName {
         MGLImage *imageToLoad = [self.delegate mapView:self didFailToLoadImage:imageName];
         if (imageToLoad) {
             auto image = [imageToLoad mgl_styleImageWithIdentifier:imageName];
-            _mbglMap->getStyle().addImage(std::move(image));
+            self.mbglMap.getStyle().addImage(std::move(image));
         }
     }
 }

platform/macos/src/MGLMapView.mm

@@ -154,7 +154,7 @@ @interface MGLMapView () <NSPopoverDelegate, MGLMultiPointDelegate, NSGestureRec
 
 @implementation MGLMapView {
     /// Cross-platform map view controller.
-    mbgl::Map *_mbglMap;
+    std::unique_ptr<mbgl::Map> _mbglMap;
     std::unique_ptr<MGLMapViewImpl> _mbglView;
     std::unique_ptr<MGLRenderFrontend> _rendererFrontend;
 
@@ -295,7 +295,7 @@ - (void)commonInit {
     resourceOptions.withCachePath([[MGLOfflineStorage sharedOfflineStorage] mbglCachePath])
                    .withAssetPath([NSBundle mainBundle].resourceURL.path.UTF8String);
 
-    _mbglMap = new mbgl::Map(*_rendererFrontend, *_mbglView, mapOptions, resourceOptions);
+    _mbglMap = std::make_unique<mbgl::Map>(*_rendererFrontend, *_mbglView, mapOptions, resourceOptions);
 
     // Notify map object when network reachability status changes.
     _reachability = [MGLReachability reachabilityForInternetConnection];
@@ -539,10 +539,7 @@ - (void)dealloc {
     // Removing the annotations unregisters any outstanding KVO observers.
     [self removeAnnotations:self.annotations];
 
-    if (_mbglMap) {
-        delete _mbglMap;
-        _mbglMap = nullptr;
-    }
+    _mbglMap.reset();
     _mbglView.reset();
 }
 
@@ -662,10 +659,6 @@ - (void)setPrefetchesTiles:(BOOL)prefetchesTiles
     _mbglMap->setPrefetchZoomDelta(prefetchesTiles ? mbgl::util::DEFAULT_PREFETCH_ZOOM_DELTA : 0);
 }
 
-- (mbgl::Map *)mbglMap {
-    return _mbglMap;
-}
-
 - (mbgl::Renderer *)renderer {
     return _rendererFrontend->getRenderer();
 }

platform/macos/src/MGLMapView_Private.h

@@ -55,8 +55,6 @@ namespace mbgl {
 
 - (BOOL)isTargetingInterfaceBuilder;
 
-- (nonnull mbgl::Map *)mbglMap;
-
 - (nonnull mbgl::Renderer *)renderer;
 
 @end