Crashes when removing and adding annotations on regionDidChangeAnimated

[Zakay]

Purpose of adding/removing annotations on regionDidChange is very clear, user pans the map and we update the result set for that region.

With a simple test, I can reproduce a crash every time.

Patch included, repositions the map on viewDidLoad, and then on regionDidChange is removes all annotations and re-adds a random amount of annotations.

Steps to reproduce:

1. Start the app, wait till annotations are visible
2. Change layer, wait for it to load
3. Move the map, around that area, back and forth until crash.

(Ignore that some annotations aren't visible after switching layers, there is already a issue reported on this matter, in our own fork we've solved this issue temporarily)

diff --git a/ios/app/MBXViewController.mm b/ios/app/MBXViewController.mm
index d05d142..d325623 100644
--- a/ios/app/MBXViewController.mm
+++ b/ios/app/MBXViewController.mm
@@ -83,6 +83,13 @@ static NSUInteger const kStyleVersion = 8;
                                                                              action:@selector(locateUser)];

     [self restoreState:nil];
+
+    [self.mapView setCenterCoordinate:CLLocationCoordinate2DMake(38.904722, -77.016389)
+                            zoomLevel:10
+                             animated:NO];
+
+    [self.mapView setDirection:0];
+
 }

 - (void)saveState:(__unused NSNotification *)notification
@@ -285,12 +295,6 @@ static NSUInteger const kStyleVersion = 8;
             dispatch_async(dispatch_get_main_queue(), ^
             {
                 [self.mapView addAnnotations:annotations];
-
-                [self.mapView setCenterCoordinate:CLLocationCoordinate2DMake(38.904722, -77.016389)
-                                        zoomLevel:10
-                                         animated:NO];
-
-                [self.mapView setDirection:0];
             });
         }
     });
@@ -321,6 +325,13 @@ static NSUInteger const kStyleVersion = 8;
     [titleButton setTitle:styleName forState:UIControlStateNormal];
 }

+- (void)mapView:(MGLMapView *__nonnull)mapView regionDidChangeAnimated:(BOOL)animated
+{
+    NSLog(@"regionDidChangeAnimated:%d", animated);
+    [mapView removeAnnotations:self.mapView.annotations];
+    [self parseFeaturesAddingCount:10 + arc4random() % 300];
+}
+
 - (void)locateUser
 {
     MGLUserTrackingMode nextMode;

[friedbunny]

@Zakay Would you mind posting the crash log here, as well? Thanks.

[Zakay]

@friedbunny Some crashlogs:
https://www.dropbox.com/sh/kbcg003g6zzfg91/AAA0XnQ4t76dRoIR1ltlRPYga

[friedbunny]

Looks like SymbolBucket::needsDependencies is being passed a bogus string.

Thread 18 name:  Worker
Thread 18 Crashed:
0   Mapbox GL  std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > mbgl::util::replaceTokens<mbgl::SymbolBucket::needsDependencies(mbgl::GeometryTileLayer const&, mapbox::util::variant<mbgl::NullExpression, mbgl::EqualsExpression, mbgl::NotEqualsExpression, mbgl::LessThanExpression, mbgl::LessThanEqualsExpression, mbgl::GreaterThanExpression, mbgl::GreaterThanEqualsExpression, mbgl::InExpression, mbgl::NotInExpression, mbgl::AnyExpression, mbgl::AllExpression, mbgl::NoneExpression> const&, mbgl::GlyphStore&, mbgl::Sprite&)::$_0>(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, mbgl::SymbolBucket::needsDependencies(mbgl::GeometryTileLayer const&, mapbox::util::variant<mbgl::NullExpression, mbgl::EqualsExpression, mbgl::NotEqualsExpression, mbgl::LessThanExpression, mbgl::LessThanEqualsExpression, mbgl::GreaterThanExpression, mbgl::GreaterThanEqualsExpression, mbgl::InExpression, mbgl::NotInExpression, mbgl::AnyExpression, mbgl::AllExpression, mbgl::NoneExpression> const&, mbgl::GlyphStore&, mbgl::Sprite&)::$_0 const&) (symbol_bucket.cpp:117)
1   Mapbox GL  std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > mbgl::util::replaceTokens<mbgl::SymbolBucket::needsDependencies(mbgl::GeometryTileLayer const&, mapbox::util::variant<mbgl::NullExpression, mbgl::EqualsExpression, mbgl::NotEqualsExpression, mbgl::LessThanExpression, mbgl::LessThanEqualsExpression, mbgl::GreaterThanExpression, mbgl::GreaterThanEqualsExpression, mbgl::InExpression, mbgl::NotInExpression, mbgl::AnyExpression, mbgl::AllExpression, mbgl::NoneExpression> const&, mbgl::GlyphStore&, mbgl::Sprite&)::$_0>(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&, mbgl::SymbolBucket::needsDependencies(mbgl::GeometryTileLayer const&, mapbox::util::variant<mbgl::NullExpression, mbgl::EqualsExpression, mbgl::NotEqualsExpression, mbgl::LessThanExpression, mbgl::LessThanEqualsExpression, mbgl::GreaterThanExpression, mbgl::GreaterThanEqualsExpression, mbgl::InExpression, mbgl::NotInExpression, mbgl::AnyExpression, mbgl::AllExpression, mbgl::NoneExpression> const&, mbgl::GlyphStore&, mbgl::Sprite&)::$_0 const&) (token.hpp:27)
2   Mapbox GL  mbgl::SymbolBucket::needsDependencies(mbgl::GeometryTileLayer const&, mapbox::util::variant<mbgl::NullExpression, mbgl::EqualsExpression, mbgl::NotEqualsExpression, mbgl::LessThanExpression, mbgl::LessThanEqualsExpression, mbgl::GreaterThanExpression, mbgl::GreaterThanEqualsExpression, mbgl::InExpression, mbgl::NotInExpression, mbgl::AnyExpression, mbgl::AllExpression, mbgl::NoneExpression> const&, mbgl::GlyphStore&, mbgl::Sprite&) (string:2438)
3   Mapbox GL  mbgl::TileWorker::createSymbolBucket(mbgl::GeometryTileLayer const&, mbgl::StyleBucket const&) (tile_worker.cpp:266)
4   Mapbox GL  mbgl::TileWorker::parseLayer(mbgl::StyleLayer const&, mbgl::GeometryTile const&) (tile_worker.cpp:151)
5   Mapbox GL  mbgl::TileWorker::parse(mbgl::GeometryTile const&) (vector:1468)
6   Mapbox GL  mbgl::Worker::Impl::parseLiveTile(mbgl::TileWorker*, mbgl::LiveTile const*, std::__1::function<void (mapbox::util::variant<mbgl::TileData::State, std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > >)>) (functional:1790)

/cc @jfirebaugh

[Zakay]

Has something been updated these last couple of days in the underlying code for this issue? I just merged from upstream, and I still haven't been able to reproduce the bug which I was able to 100% of the times before. I'm going to keep on testing, if I'm not able to reproduce it by the end of the day, then this issue can be closed.

[friedbunny]

This may have been addressed incidentally by #2617.

[Zakay]

Yes, this is now fixed since #2617