mapbox / mapbox-gl-native

Interactive, thoroughly customizable maps in native Android, iOS, macOS, Node.js, and Qt applications, powered by vector tiles and OpenGL
https://mapbox.com/mobile
Other
4.37k stars 1.33k forks source link

[ios] Crash on method removeSource: in MGLSource class #9384

Closed MickzeKick closed 7 years ago

MickzeKick commented 7 years ago

Platform:iOS Mapbox SDK version:3.6.0 rc1

  1. Add 3 MGLShapeSource on mapView, then insert 3 MGLLineStyleLayer on mapView.
  2. Remove this 3 MGLLineStyleLayer in a for loop, the same things for 3 MGLShapeSource in a for loop too.
  3. Then application Crash on removeSource: method ([self.mapView.style removeSource:source] in a for loop) but with the same code the crash doesn't exists in release version iOS 3.5.2.

Expected behavior

Remove Polylines

Actual behavior

Application Crash

frederoni commented 7 years ago

Thank you for your report. In what order do you remove the layers and sources? Can you include the relevant part of your implementation? Do you get any errors when the application crashes?

fabian-guerra commented 7 years ago

I'm able to reproduce this issue performing the steps described above. It was introduced since the alpha release. The problem rises in [MGLSource removeFromMapView:] I'm tracking which change introduced the regression.

/cc @boundsj @1ec5 @jfirebaugh

1ec5 commented 7 years ago

Wonder if #8626 is related.

1ec5 commented 7 years ago

Also reproduces in macosapp with this modification:

diff --git a/platform/macos/app/MapDocument.m b/platform/macos/app/MapDocument.m
index 59844d363..00a9e4cc9 100644
--- a/platform/macos/app/MapDocument.m
+++ b/platform/macos/app/MapDocument.m
@@ -1055,6 +1055,35 @@ NS_ARRAY_OF(id <MGLAnnotation>) *MBXFlattenedShapes(NS_ARRAY_OF(id <MGLAnnotatio

 - (void)mapView:(MGLMapView *)mapView didFinishLoadingStyle:(MGLStyle *)style {
     [self updateLabels];
+    
+    CLLocationCoordinate2D coordinates[] = {
+        WorldTourDestinations[0],
+        WorldTourDestinations[1],
+        WorldTourDestinations[2],
+        WorldTourDestinations[3],
+    };
+    MGLPolylineFeature *point = [MGLPolylineFeature polylineWithCoordinates:coordinates count:sizeof(coordinates) / sizeof(coordinates[0])];
+    MGLShapeSource *one = [[MGLShapeSource alloc] initWithIdentifier:@"one" shape:point options:nil];
+    MGLShapeSource *two = [[MGLShapeSource alloc] initWithIdentifier:@"two" shape:point options:nil];
+    MGLShapeSource *three = [[MGLShapeSource alloc] initWithIdentifier:@"three" shape:point options:nil];
+    [style addSource:one];
+    [style addSource:two];
+    [style addSource:three];
+    
+    MGLLineStyleLayer *a = [[MGLLineStyleLayer alloc] initWithIdentifier:@"a" source:one];
+    MGLLineStyleLayer *bee = [[MGLLineStyleLayer alloc] initWithIdentifier:@"bee" source:two];
+    MGLLineStyleLayer *cee = [[MGLLineStyleLayer alloc] initWithIdentifier:@"cee" source:three];
+    [style addLayer:a];
+    [style addLayer:bee];
+    [style addLayer:cee];
+    
+    [style removeLayer:a];
+    [style removeLayer:bee];
+    [style removeLayer:cee];
+    
+    [style removeSource:one];
+    [style removeSource:two];
+    [style removeSource:three];
 }

 - (BOOL)mapView:(MGLMapView *)mapView annotationCanShowCallout:(id <MGLAnnotation>)annotation {

and this stack trace:

Thread 1Queue : com.apple.main-thread (serial)
#0  0x0000000100a54cf4 in auto mbgl::style::Style::removeSource(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&)::$_3::operator()<std::__1::unique_ptr<mbgl::RenderSource, std::__1::default_delete<mbgl::RenderSource> > >(std::__1::unique_ptr<mbgl::RenderSource, std::__1::default_delete<mbgl::RenderSource> > const&) const at /path/to/mapbox-gl-native/src/mbgl/style/style.cpp:232
#1  0x0000000100a54561 in void mbgl::util::erase_if<std::__1::vector<std::__1::unique_ptr<mbgl::RenderSource, std::__1::default_delete<mbgl::RenderSource> >, std::__1::allocator<std::__1::unique_ptr<mbgl::RenderSource, std::__1::default_delete<mbgl::RenderSource> > > >, std::__1::__wrap_iter<std::__1::unique_ptr<mbgl::RenderSource, std::__1::default_delete<mbgl::RenderSource> >*>, mbgl::style::Style::removeSource(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&)::$_3>(std::__1::vector<std::__1::unique_ptr<mbgl::RenderSource, std::__1::default_delete<mbgl::RenderSource> >, std::__1::allocator<std::__1::unique_ptr<mbgl::RenderSource, std::__1::default_delete<mbgl::RenderSource> > > >&, std::__1::__wrap_iter<std::__1::unique_ptr<mbgl::RenderSource, std::__1::default_delete<mbgl::RenderSource> >*>, std::__1::__wrap_iter<std::__1::unique_ptr<mbgl::RenderSource, std::__1::default_delete<mbgl::RenderSource> >*>, mbgl::style::Style::removeSource(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&)::$_3) at /path/to/mapbox-gl-native/src/mbgl/util/std.hpp:13
#2  0x0000000100a2d7d5 in void mbgl::util::erase_if<std::__1::vector<std::__1::unique_ptr<mbgl::RenderSource, std::__1::default_delete<mbgl::RenderSource> >, std::__1::allocator<std::__1::unique_ptr<mbgl::RenderSource, std::__1::default_delete<mbgl::RenderSource> > > >, mbgl::style::Style::removeSource(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&)::$_3>(std::__1::vector<std::__1::unique_ptr<mbgl::RenderSource, std::__1::default_delete<mbgl::RenderSource> >, std::__1::allocator<std::__1::unique_ptr<mbgl::RenderSource, std::__1::default_delete<mbgl::RenderSource> > > >&, mbgl::style::Style::removeSource(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&)::$_3) at /path/to/mapbox-gl-native/src/mbgl/util/std.hpp:23
#3  0x0000000100a2ca08 in mbgl::style::Style::removeSource(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) at /path/to/mapbox-gl-native/src/mbgl/style/style.cpp:231
#4  0x00000001005f4f17 in mbgl::Map::removeSource(std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char> > const&) at /path/to/mapbox-gl-native/src/mbgl/map/map.cpp:895
#5  0x0000000100259488 in ::-[MGLSource removeFromMapView:](MGLMapView *) at /path/to/mapbox-gl-native/platform/darwin/src/MGLSource.mm:59
#6  0x000000010021e147 in ::-[MGLStyle removeSource:](MGLSource *) at /path/to/mapbox-gl-native/platform/darwin/src/MGLStyle.mm:210
#7  0x000000010000f8f8 in -[MapDocument mapView:didFinishLoadingStyle:] at /path/to/mapbox-gl-native/platform/macos/app/MapDocument.m:1084

Reproduces if the sources are removed asynchronously after a delay. Reproduces if -[MGLStyle layerWithIdentifier:] and -[MGLStyle sourceWithIdentifier:] are called instead of reusing the local variables representing the layers and sources.

fabian-guerra commented 7 years ago

Fixed in https://github.com/mapbox/mapbox-gl-native/issues/9388

MickzeKick commented 7 years ago

Hello,

Thank-you for your prompt response and corrective. 👍 Yes it's in the method removeFromMapView from MGLSource class where the Crash appear but I haven't Stack Trace. So yesterday I try to debugging but I realized that the bug is in the C++ code, so I prefer to create an issue.

Thanks again.

++

MzK