[ios] Added sets to manage retain/release of layers (esp custom gl layers)

6 files changed, 89 insertions, 12 deletions

diff --git a/platform/darwin/src/MGLOpenGLStyleLayer.mm b/platform/darwin/src/MGLOpenGLStyleLayer.mm
index 9f957b7082..80805b001b 100644
--- a/platform/darwin/src/MGLOpenGLStyleLayer.mm
+++ b/platform/darwin/src/MGLOpenGLStyleLayer.mm
@@ -15,10 +15,9 @@
     when creating an OpenGL style layer.
  */
 void MGLPrepareCustomStyleLayer(void *context) {
-
-    // Pair retain/release during rendering (see MGLFinishCustomStyleLayer)
-    id retaineee = (__bridge id)context;
-    MGLOpenGLStyleLayer *layer = (__bridge MGLOpenGLStyleLayer*)CFBridgingRetain(retaineee);
+    // Note, that the layer is retained/released by MGLStyle, ensuring that the layer
+    // is alive during rendering
+    MGLOpenGLStyleLayer *layer = (__bridge MGLOpenGLStyleLayer*)context;
 
     [layer didMoveToMapView:layer.style.mapView];
 }
@@ -51,8 +50,10 @@ void MGLDrawCustomStyleLayer(void *context, const mbgl::style::CustomLayerRender
     when creating an OpenGL style layer.
  */
 void MGLFinishCustomStyleLayer(void *context) {
-    // Release the layer (since we retained it in the initialization)
-    MGLOpenGLStyleLayer *layer = CFBridgingRelease(context);
+
+    // Note, that the layer is retained/released by MGLStyle, ensuring that the layer
+    // is alive during rendering
+    MGLOpenGLStyleLayer *layer = (__bridge MGLOpenGLStyleLayer*)context;
     [layer willMoveFromMapView:layer.style.mapView];
 }
 
diff --git a/platform/darwin/src/MGLStyle.mm b/platform/darwin/src/MGLStyle.mm
index f6fc5533be..695f8b55e7 100644
--- a/platform/darwin/src/MGLStyle.mm
+++ b/platform/darwin/src/MGLStyle.mm
@@ -85,6 +85,10 @@
 @property (readonly, copy, nullable) NSURL *URL;
 @property (nonatomic) NS_MUTABLE_DICTIONARY_OF(NSString *, NS_DICTIONARY_OF(NSObject *, MGLTextLanguage *) *) *localizedLayersByIdentifier;
 
+// Used for retain/release management
+@property (nonatomic) NSMutableSet *layersForUpdating;
+@property (nonatomic) NSSet *layersForRendering;
+
 @end
 
 @implementation MGLStyle
@@ -172,6 +176,8 @@ static NSURL *MGLStyleURL_trafficNight;
 
 - (instancetype)initWithRawStyle:(mbgl::style::Style *)rawStyle mapView:(MGLMapView *)mapView {
     if (self = [super init]) {
+        _layersForUpdating = [NSMutableSet set];
+
         _mapView = mapView;
         _rawStyle = rawStyle;
         _localizedLayersByIdentifier = [NSMutableDictionary dictionary];
@@ -534,6 +540,25 @@ static NSURL *MGLStyleURL_trafficNight;
     [self didChangeValueForKey:@"layers"];
 }
 
+#pragma mark - Layer retain/release management
+
+- (void)addToManagedLayers:(MGLStyleLayer*)layer {
+    [self.layersForUpdating addObject:layer];
+}
+
+- (void)removeFromManagedLayers:(MGLStyleLayer*)layer {
+    [self.layersForUpdating removeObject:layer];
+}
+
+- (void)retainLayersUsedDuringRendering {
+    self.layersForRendering = [self.layersForUpdating copy];
+}
+
+- (void)releaseLayersUsedDuringRendering {
+    self.layersForRendering = nil;
+}
+
+
 #pragma mark Style classes
 
 - (NS_ARRAY_OF(NSString *) *)styleClasses
diff --git a/platform/darwin/src/MGLStyleLayer.mm b/platform/darwin/src/MGLStyleLayer.mm
index 45a4486525..444dbaeea3 100644
--- a/platform/darwin/src/MGLStyleLayer.mm
+++ b/platform/darwin/src/MGLStyleLayer.mm
@@ -38,9 +38,9 @@
                     "to the style more than once is invalid.", self, style];
     }
 
-    // Since we're adding self to a C++ collection, we need to retain ourselves, so that we don't
-    // end up with a dangling pointer
-    CFBridgingRetain(self);
+    // We need to ensure that this layer is retained, so that any references from layer impl's
+    // e.g. contexts) are still valid
+    [style addToManagedLayers:self];
 
     if (otherLayer) {
         const mbgl::optional<std::string> belowLayerId{otherLayer.identifier.UTF8String};
@@ -55,9 +55,9 @@
     if (self.rawLayer == style.rawStyle->getLayer(self.identifier.UTF8String)) {
         _pendingLayer = style.rawStyle->removeLayer(self.identifier.UTF8String);
 
-        // Pair the retain above, and release self, since we're now removed from the collection
-        CFTypeRef toRelease = (__bridge CFTypeRef)self;
-        CFBridgingRelease(toRelease);
+        // We need to ensure that this layer is now released (however, if this layer is about to be
+        // used by the renderer then it will released once rendering is complete)
+        [style removeFromManagedLayers:self];
     }
 }
 
diff --git a/platform/darwin/src/MGLStyle_Private.h b/platform/darwin/src/MGLStyle_Private.h
index 4cbe953a44..cfde78fd06 100644
--- a/platform/darwin/src/MGLStyle_Private.h
+++ b/platform/darwin/src/MGLStyle_Private.h
@@ -28,6 +28,10 @@ namespace mbgl {
 
 - (void)setStyleClasses:(NS_ARRAY_OF(NSString *) *)appliedClasses transitionDuration:(NSTimeInterval)transitionDuration;
 
+- (void)addToManagedLayers:(MGLStyleLayer*)layer;
+- (void)removeFromManagedLayers:(MGLStyleLayer*)layer;
+- (void)retainLayersUsedDuringRendering;
+- (void)releaseLayersUsedDuringRendering;
 @end
 
 @interface MGLStyle (MGLStreetsAdditions)
diff --git a/platform/ios/Integration Tests/MBGLIntegrationTests.m b/platform/ios/Integration Tests/MBGLIntegrationTests.m
index cb3bdc3df6..ea95d25c98 100644
--- a/platform/ios/Integration Tests/MBGLIntegrationTests.m
+++ b/platform/ios/Integration Tests/MBGLIntegrationTests.m
@@ -142,6 +142,43 @@
     XCTAssertNil(weakLayer);
 }
 
+- (void)testOpenGLLayerDoesNotLeakWhenMapViewDeallocs {
+    NSTimeInterval waitInterval = 0.02;
+    __weak id weakLayer;
+
+    @autoreleasepool {
+
+        NSURL *styleURL = [[NSBundle bundleForClass:[self class]] URLForResource:@"one-liner" withExtension:@"json"];
+        MGLMapView *mapView2 = [[MGLMapView alloc] initWithFrame:UIScreen.mainScreen.bounds styleURL:styleURL];
+        mapView2.delegate = self;
+
+        XCTAssertNil(mapView2.style);
+
+        _styleLoadingExpectation = [self expectationWithDescription:@"Map view should finish loading style."];
+        [self waitForExpectationsWithTimeout:1 handler:nil];
+
+//        UIView *superView = [[UIView alloc] initWithFrame:UIScreen.mainScreen.bounds];
+//        [superView addSubview:self.mapView];
+//        UIWindow *window = [[UIWindow alloc] initWithFrame:UIScreen.mainScreen.bounds];
+//        [window addSubview:superView];
+//        [window makeKeyAndVisible];
+
+        MGLOpenGLStyleLayer *layer = [[MGLOpenGLStyleLayer alloc] initWithIdentifier:@"gl-layer"];
+        weakLayer = layer;
+        [mapView2.style insertLayer:layer atIndex:0];
+        layer = nil;
+
+        [[NSRunLoop currentRunLoop] runUntilDate:[[NSDate date] dateByAddingTimeInterval:waitInterval]];
+        [self.mapView setNeedsDisplay];
+        [[NSRunLoop currentRunLoop] runUntilDate:[[NSDate date] dateByAddingTimeInterval:waitInterval]];
+    }
+    XCTAssertNil(weakLayer);
+}
+
+
+
+
+
 - (void)testOpenGLLayerDoesNotLeakWhenRemovedFromStyle {
     NSTimeInterval waitInterval = 0.02;
 
diff --git a/platform/ios/src/MGLMapView.mm b/platform/ios/src/MGLMapView.mm
index cad14343a8..2df7de2c5c 100644
--- a/platform/ios/src/MGLMapView.mm
+++ b/platform/ios/src/MGLMapView.mm
@@ -201,6 +201,7 @@ public:
 
 @property (nonatomic, readwrite) MGLStyle *style;
 
+
 @property (nonatomic) UITapGestureRecognizer *singleTapGestureRecognizer;
 @property (nonatomic) UITapGestureRecognizer *doubleTap;
 @property (nonatomic) UITapGestureRecognizer *twoFingerTap;
@@ -359,7 +360,10 @@ public:
     }
 
     styleURL = styleURL.mgl_URLByStandardizingScheme;
+
+//  [self.layers removeAllObjects];
     self.style = nil;
+
     _mbglMap->getStyle().loadURL([[styleURL absoluteString] UTF8String]);
 }
 
@@ -5549,10 +5553,13 @@ public:
 }
 
 - (void)mapViewWillStartRenderingFrame {
+
     if (!_mbglMap) {
         return;
     }
 
+    [self.style retainLayersUsedDuringRendering];
+
     if ([self.delegate respondsToSelector:@selector(mapViewWillStartRenderingFrame:)])
     {
         [self.delegate mapViewWillStartRenderingFrame:self];
@@ -5575,6 +5582,9 @@ public:
     {
         [self.delegate mapViewDidFinishRenderingFrame:self fullyRendered:fullyRendered];
     }
+
+    [self.style releaseLayersUsedDuringRendering];
+
 }
 
 - (void)mapViewWillStartRenderingMap {